{"thread":{"id":"32049","subject":"What's cooking in git.git (Nov 2012, #02; Fri, 9)","startedAt":"2012-11-09T19:23:36Z","lastAt":"2012-11-30T20:24:35Z","messageCount":38,"participants":["Jeff King","Ralf Thielow","Junio C Hamano","Kalle Olavi Niemitalo","Felipe Contreras","SZEDER Gábor","Paul Fox","Andreas Schwab","Krzysztof Mazur","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"202719","messageId":"20121109192336.GA9401@sigill.intra.peff.net","threadId":"32049","inReplyTo":null,"subject":"What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-09T19:23:36Z","receivedAt":"2012-11-09T19:23:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"What's cooking in git.git (Nov 2012, #02; Fri, 9)\n--------------------------------------------------\n\nHere 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'.\n\nThe fourth batch of topics has graduated to master. This should be my\nlast integration cycle, as Junio will be back to take over before the\nnext one.\n\nYou can find the changes described here in the integration branches of\nmy repository at:\n\n  git://github.com/peff/git.git\n\nUntil Junio returns, kernel.org and the other \"usual\" places will not be\nupdated.\n\n--------------------------------------------------\n[New Topics]\n\n* cr/push-force-tag-update (2012-11-09) 5 commits\n - push: update remote tags only with force\n - push: flag updates that require force\n - push: flag updates\n - push: add advice for rejected tag reference\n - push: return reject reasons via a mask\n\n Require \"-f\" for push to update a tag, even if it is a fast-forward.\n\n Needs review.\n\n I'm undecided yet on whether the goal is the right thing to do, but it\n does prevent some potential mistakes. I haven't looked closely at the\n implementation itself; review from interested parties would be helpful.\n\n\n* fc/fast-export-fixes (2012-11-08) 14 commits\n - fast-export: don't handle uninteresting refs\n - fast-export: make sure updated refs get updated\n - fast-export: fix comparison in tests\n - fast-export: trivial cleanup\n - remote-testgit: make clear the 'done' feature\n - remote-testgit: report success after an import\n - remote-testgit: exercise more features\n - remote-testgit: cleanup tests\n - remote-testgit: remove irrelevant test\n - remote-testgit: get rid of non-local functionality\n - Add new simplified git-remote-testgit\n - Rename git-remote-testgit to git-remote-testpy\n - remote-testgit: fix direction of marks\n - fast-export: avoid importing blob marks\n\n Improvements to fix fast-export bugs, including how refs pointing to\n already-seen commits are handled. An earlier 4-commit version of this\n series looked good to me, but this much-expanded version has not seen\n any comments.\n\n Needs review.\n\n\n* mg/maint-pull-suggest-upstream-to (2012-11-08) 1 commit\n - push/pull: adjust missing upstream help text to changed interface\n\n Follow-on to the new \"--set-upstream-to\" topic from v1.8.0 to avoid\n suggesting the deprecated \"--set-upstream\".\n\n Will merge to 'next'.\n\n\n* mh/alt-odb-string-list-cleanup (2012-11-08) 2 commits\n - link_alt_odb_entries(): take (char *, len) rather than two pointers\n - link_alt_odb_entries(): use string_list_split_in_place()\n\n Cleanups in the alternates code. Fixes a potential bug and makes the\n code much cleaner.\n\n Will merge to 'next'.\n\n\n* pf/editor-ignore-sigint (2012-11-08) 1 commit\n - launch_editor: ignore SIGINT while the editor has control\n\n Avoid confusing cases where the user hits Ctrl-C while in the editor\n session, not realizing git will receive the signal. Since most editors\n will take over the terminal and will block SIGINT, this is not likely\n to confuse anyone.\n\n Comments welcome from people using unusual editors (e.g., a script that\n starts an editor in another window then blocks, waiting for the user to\n finish).\n\n\n* pp/gitweb-config-underscore (2012-11-08) 1 commit\n - gitweb: make remote_heads config setting work\n\n The key \"gitweb.remote_heads\" is not legal git config; this maps it to\n \"gitweb.remoteheads\".\n\n Junio raised a good point about the implementation for three-level\n variables.\n\n Expecting a re-roll.\n\n\n* pw/maint-p4-rcs-expansion-newline (2012-11-08) 1 commit\n - git p4: RCS expansion should not span newlines\n\n I do not have p4 to play with, but looks obviously correct to me.\n\n Will merge to 'next'.\n\n\n* rh/maint-gitweb-highlight-ext (2012-11-08) 1 commit\n - gitweb.perl: fix %highlight_ext mappings\n\n Fixes a clever misuse of perl's list interpretation.\n\n Will merge to 'next'.\n\n\n* rr/submodule-diff-config (2012-11-08) 3 commits\n - submodule: display summary header in bold\n - diff: introduce diff.submodule configuration variable\n - Documentation: move diff.wordRegex from config.txt to diff-config.txt\n\n Lets \"git diff --submodule=log\" become the default via configuration.\n\n Seems like a good direction, though I had a few comments.\n\n Expecting a re-roll.\n\n\n--------------------------------------------------\n[Graduated to \"master\"]\n\n* fc/completion-send-email-with-format-patch (2012-10-16) 1 commit\n  (merged to 'next' on 2012-11-04 at 0a6366e)\n + completion: add format-patch options to send-email\n\n Will merge to 'master' in the fourth batch.\n\n\n* js/format-2047 (2012-10-18) 7 commits\n  (merged to 'next' on 2012-10-25 at 76d91fe)\n + format-patch tests: check quoting/encoding in To: and Cc: headers\n + format-patch: fix rfc2047 address encoding with respect to rfc822 specials\n + format-patch: make rfc2047 encoding more strict\n + format-patch: introduce helper function last_line_length()\n + format-patch: do not wrap rfc2047 encoded headers too late\n + format-patch: do not wrap non-rfc2047 headers too early\n + utf8: fix off-by-one wrapping of text\n\n Fixes many rfc2047 quoting issues in the output from format-patch.\n\n Will merge to 'master' in the fourth batch.\n\n\n* km/send-email-compose-encoding (2012-10-25) 5 commits\n  (merged to 'next' on 2012-10-29 at d7d2bb4)\n + git-send-email: add rfc2047 quoting for \"=?\"\n + git-send-email: introduce quote_subject()\n + git-send-email: skip RFC2047 quoting for ASCII subjects\n + git-send-email: use compose-encoding for Subject\n  (merged to 'next' on 2012-10-25 at 5447367)\n + git-send-email: introduce compose-encoding\n\n \"git send-email --compose\" can let the user create a non-ascii\n cover letter message, but there was not a way to mark it with\n appropriate content type before sending it out.\n\n Further updates fix subject quoting.\n\n Will merge to 'master' in the fourth batch.\n\n\n* mh/maint-parse-dirstat-fix (2012-10-29) 1 commit\n  (merged to 'next' on 2012-11-04 at 852d609)\n + parse_dirstat_params(): use string_list to split comma-separated string\n\n Cleans up some code and avoids a potential bug.\n\n Will merge to 'master' in the fourth batch.\n\n\n* mo/cvs-server-cleanup (2012-10-26) 11 commits\n  (merged to 'next' on 2012-10-29 at 4e70622)\n + Use character class for sed expression instead of \\s\n  (merged to 'next' on 2012-10-25 at c70881d)\n + cvsserver status: provide real sticky info\n + cvsserver: cvs add: do not expand directory arguments\n + cvsserver: use whole CVS rev number in-process; don't strip \"1.\" prefix\n + cvsserver: split up long lines in req_{status,diff,log}\n + cvsserver: clean up client request handler map comments\n + cvsserver: remove unused functions _headrev and gethistory\n + cvsserver update: comment about how we shouldn't remove a user-modified file\n + cvsserver: add comments about database schema/usage\n + cvsserver: removed unused sha1Or-k mode from kopts_from_path\n + cvsserver t9400: add basic 'cvs log' test\n (this branch is tangled with mo/cvs-server-updates.)\n\n Cleanups to prepare for mo/cvs-server-updates.\n\n Will merge to 'master' in the fourth batch.\n\n\n* nd/attr-match-optim-more (2012-10-15) 7 commits\n  (merged to 'next' on 2012-10-25 at 09f70ce)\n + attr: more matching optimizations from .gitignore\n + gitignore: make pattern parsing code a separate function\n + exclude: split pathname matching code into a separate function\n + exclude: fix a bug in prefix compare optimization\n + exclude: split basename matching code into a separate function\n + exclude: stricten a length check in EXC_FLAG_ENDSWITH case\n + Merge commit 'f9f6e2c' into nd/attr-match-optim-more\n (this branch is used by as/check-ignore and nd/wildmatch.)\n\n Start laying the foundation to build the \"wildmatch\" after we can\n agree on its desired semantics.\n\n Will merge to 'master' in the fourth batch.\n\n\n* nd/builtin-to-libgit (2012-10-29) 7 commits\n  (merged to 'next' on 2012-11-04 at 06cbf9b)\n + fetch-pack: move core code to libgit.a\n + fetch-pack: remove global (static) configuration variable \"args\"\n + send-pack: move core code to libgit.a\n + Move setup_diff_pager to libgit.a\n + Move print_commit_list to libgit.a\n + Move estimate_bisect_steps to libgit.a\n + Move try_merge_command and checkout_fast_forward to libgit.a\n\n Code cleanups so that libgit.a does not depend on anything in the\n builtin/ directory.\n\n Some of the code movement is pretty big, but there doesn't seem to be\n any conflicts with topics in flight.\n\n Will merge to 'master' in the fourth batch.\n\n\n* nd/tree-walk-enum-cleanup (2012-10-19) 1 commit\n  (merged to 'next' on 2012-11-04 at 8ccdf98)\n + tree-walk: use enum interesting instead of integer\n\n Will merge to 'master' in the fourth batch.\n\n\n* ph/maint-submodule-status-fix (2012-10-29) 2 commits\n  (merged to 'next' on 2012-11-04 at d700e02)\n + submodule status: remove unused orig_* variables\n + t7407: Fix recursive submodule test\n\n Cleans up some leftover bits from an earlier submodule change.\n\n Will merge to 'master' in the fourth batch.\n\n\n* rs/lock-correct-ref-during-delete (2012-10-16) 1 commit\n  (merged to 'next' on 2012-10-25 at 9341eea)\n + refs: lock symref that is to be deleted, not its target\n\n When \"update-ref -d --no-deref SYM\" tried to delete a symbolic ref\n SYM, it incorrectly locked the underlying reference pointed by SYM,\n not the symbolic ref itself.\n\n Will merge to 'master' in the fourth batch.\n\n\n* sz/maint-curl-multi-timeout (2012-10-19) 1 commit\n  (merged to 'next' on 2012-11-04 at f696dd8)\n + Fix potential hang in https handshake\n\n Sometimes curl_multi_timeout() function suggested a wrong timeout\n value when there is no file descriptors to wait on and the http\n transport ended up sleeping for minutes in select(2) system call.\n Detect this and reduce the wait timeout in such a case.\n\n Will merge to 'master' in the fourth batch.\n\n--------------------------------------------------\n[Stalled]\n\n* rc/maint-complete-git-p4 (2012-09-24) 1 commit\n  (merged to 'next' on 2012-10-29 at af52cef)\n + Teach git-completion about git p4\n\n Comment from Pete will need to be addressed in a follow-up patch.\n\n\n* as/test-tweaks (2012-09-20) 7 commits\n - tests: paint unexpectedly fixed known breakages in bold red\n - tests: test the test framework more thoroughly\n - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up\n - tests: refactor mechanics of testing in a sub test-lib\n - tests: paint skipped tests in bold blue\n - tests: test number comes first in 'not ok $count - $message'\n - tests: paint known breakages in bold yellow\n\n Various minor tweaks to the test framework to paint its output\n lines in colors that match what they mean better.\n\n Has the \"is this really blue?\" issue Peff raised resolved???\n\n\n* jc/maint-name-rev (2012-09-17) 7 commits\n - describe --contains: use \"name-rev --algorithm=weight\"\n - name-rev --algorithm=weight: tests and documentation\n - name-rev --algorithm=weight: cache the computed weight in notes\n - name-rev --algorithm=weight: trivial optimization\n - name-rev: --algorithm option\n - name_rev: clarify the logic to assign a new tip-name to a commit\n - name-rev: lose unnecessary typedef\n\n \"git name-rev\" names the given revision based on a ref that can be\n reached in the smallest number of steps from the rev, but that is\n not useful when the caller wants to know which tag is the oldest one\n that contains the rev.  This teaches a new mode to the command that\n uses the oldest ref among those which contain the rev.\n\n I am not sure if this is worth it; for one thing, even with the help\n from notes-cache, it seems to make the \"describe --contains\" even\n slower. Also the command will be unusably slow for a user who does\n not have a write access (hence unable to create or update the\n notes-cache).\n\n Stalled mostly due to lack of responses.\n\n\n* jc/xprm-generation (2012-09-14) 1 commit\n - test-generation: compute generation numbers and clock skews\n\n A toy to analyze how bad the clock skews are in histories of real\n world projects.\n\n Stalled mostly due to lack of responses.\n\n\n* jc/blame-no-follow (2012-09-21) 2 commits\n - blame: pay attention to --no-follow\n - diff: accept --no-follow option\n\n Teaches \"--no-follow\" option to \"git blame\" to disable its\n whole-file rename detection.\n\n Stalled mostly due to lack of responses.\n\n\n* jc/doc-default-format (2012-10-07) 2 commits\n - [SQAUSH] allow \"cd Doc* && make DEFAULT_DOC_TARGET=...\"\n - Allow generating a non-default set of documentation\n\n Need to address the installation half if this is to be any useful.\n\n\n* mk/maint-graph-infinity-loop (2012-09-25) 1 commit\n - graph.c: infinite loop in git whatchanged --graph -m\n\n The --graph code fell into infinite loop when asked to do what the\n code did not expect ;-)\n\n Anybody who worked on \"--graph\" wants to comment?\n Stalled mostly due to lack of responses.\n\n\n* jc/add-delete-default (2012-08-13) 1 commit\n - git add: notice removal of tracked paths by default\n\n \"git add dir/\" updated modified files and added new files, but does\n not notice removed files, which may be \"Huh?\" to some users.  They\n can of course use \"git add -A dir/\", but why should they?\n\n Resurrected from graveyard, as I thought it was a worthwhile thing\n to do in the longer term.\n\n Waiting for comments.\n\n\n* mb/remote-default-nn-origin (2012-07-11) 6 commits\n - Teach get_default_remote to respect remote.default.\n - Test that plain \"git fetch\" uses remote.default when on a detached HEAD.\n - Teach clone to set remote.default.\n - Teach \"git remote\" about remote.default.\n - Teach remote.c about the remote.default configuration setting.\n - Rename remote.c's default_remote_name static variables.\n\n When the user does not specify what remote to interact with, we\n often attempt to use 'origin'.  This can now be customized via a\n configuration variable.\n\n Expecting a re-roll.\n\n \"The first remote becomes the default\" bit is better done as a\n separate step.\n\n\n* mh/ceiling (2012-10-29) 8 commits\n - string_list_longest_prefix(): remove function\n - setup_git_directory_gently_1(): resolve symlinks in ceiling paths\n - longest_ancestor_length(): require prefix list entries to be normalized\n - longest_ancestor_length(): take a string_list argument for prefixes\n - longest_ancestor_length(): use string_list_split()\n - Introduce new function real_path_if_valid()\n - real_path_internal(): add comment explaining use of cwd\n - Introduce new static function real_path_internal()\n\n Elements of GIT_CEILING_DIRECTORIES list may not match the real\n pathname we obtain from getcwd(), leading the GIT_DIR discovery\n logic to escape the ceilings the user thought to have specified.\n\n\n--------------------------------------------------\n[Cooking]\n\n* mo/cvs-server-updates (2012-10-16) 10 commits\n - cvsserver Documentation: new cvs ... -r support\n - cvsserver: add t9402 to test branch and tag refs\n - cvsserver: support -r and sticky tags for most operations\n - cvsserver: Add version awareness to argsfromdir\n - cvsserver: generalize getmeta() to recognize commit refs\n - cvsserver: implement req_Sticky and related utilities\n - cvsserver: add misc commit lookup, file meta data, and file listing functions\n - cvsserver: define a tag name character escape mechanism\n - cvsserver: cleanup extra slashes in filename arguments\n - cvsserver: factor out git-log parsing logic\n\n Needs review by folks interested in cvsserver.\n\n\n* ta/doc-cleanup (2012-10-25) 6 commits\n - Documentation: build html for all files in technical and howto\n - Documentation/howto: convert plain text files to asciidoc\n - Documentation/technical: convert plain text files to asciidoc\n - Change headline of technical/send-pack-pipeline.txt to not confuse its content with content from git-send-pack.txt\n - Shorten two over-long lines in git-bisect-lk2009.txt by abbreviating some sha1\n - Split over-long synopsis in git-fetch-pack.txt into several lines\n\n Will merge to 'next'.\n\n\n* lt/diff-stat-show-0-lines (2012-10-17) 1 commit\n - Fix \"git diff --stat\" for interesting - but empty - file changes\n\n We failed to mention a file without any content change but whose\n permission bit was modified, or (worse yet) a new file without any\n content in the \"git diff --stat\" output.\n\n Needs some test updates.\n\n\n* jc/prettier-pretty-note (2012-10-26) 11 commits\n  (merged to 'next' on 2012-11-04 at 40e3e48)\n + Doc User-Manual: Patch cover letter, three dashes, and --notes\n + Doc format-patch: clarify --notes use case\n + Doc notes: Include the format-patch --notes option\n + Doc SubmittingPatches: Mention --notes option after \"cover letter\"\n + Documentation: decribe format-patch --notes\n + format-patch --notes: show notes after three-dashes\n + format-patch: append --signature after notes\n + pretty_print_commit(): do not append notes message\n + pretty: prepare notes message at a centralized place\n + format_note(): simplify API\n + pretty: remove reencode_commit_message()\n\n Now that Philip has submitted some documentation updates, this is\n looking more ready.\n\n Will merge to 'master' in the fifth batch.\n\n\n* jc/same-encoding (2012-11-04) 1 commit\n  (merged to 'next' on 2012-11-04 at 54991f2)\n + reencode_string(): introduce and use same_encoding()\n\n Various codepaths checked if two encoding names are the same using\n ad-hoc code and some of them ended up asking iconv() to convert\n between \"utf8\" and \"UTF-8\".  The former is not a valid way to spell\n the encoding name, but often people use it by mistake, and we\n equated them in some but not all codepaths. Introduce a new helper\n function to make these codepaths consistent.\n\n Will merge to 'master' in the fifth batch.\n\n\n* cr/cvsimport-local-zone (2012-11-04) 2 commits\n  (merged to 'next' on 2012-11-04 at 292f0b4)\n + cvsimport: work around perl tzset issue\n + git-cvsimport: allow author-specific timezones\n\n Allows \"cvsimport\" to read per-author timezone from the author info\n file.\n\n Will merge to 'master' in the fifth batch.\n\n\n* fc/zsh-completion (2012-10-29) 3 commits\n - completion: add new zsh completion\n - completion: add new __gitcompadd helper\n - completion: get rid of empty COMPREPLY assignments\n\n There were some comments on this, but I wasn't clear on the outcome.\n\n Need to take a closer look.\n\n\n* jc/apply-trailing-blank-removal (2012-10-12) 1 commit\n - apply.c:update_pre_post_images(): the preimage can be truncated\n\n Fix to update_pre_post_images() that did not take into account the\n possibility that whitespace fix could shrink the preimage and\n change the number of lines in it.\n\n Extra set of eyeballs appreciated.\n\n\n* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits\n - config: exit on error accessing any config file\n - doc: advertise GIT_CONFIG_NOSYSTEM\n - config: treat user and xdg config permission problems as errors\n - config, gitignore: failure to access with ENOTDIR is ok\n\n An RFC to deal with a situation where .config/git is a file and we\n notice .config/git/config is not readable due to ENOTDIR, not\n ENOENT; I think a bit more refactored approach to consistently\n address permission errors across config, exclude and attrs is\n desirable.  Don't we also need a check for an opposite situation\n where we open .config/git/config or .gitattributes for reading but\n they turn out to be directories?\n\n\n* as/check-ignore (2012-11-08) 14 commits\n - t0007: fix tests on Windows\n - Documentation/check-ignore: we show the deciding match, not the first\n - Add git-check-ignore sub-command\n - dir.c: provide free_directory() for reclaiming dir_struct memory\n - pathspec.c: move reusable code from builtin/add.c\n - dir.c: refactor treat_gitlinks()\n - dir.c: keep track of where patterns came from\n - dir.c: refactor is_path_excluded()\n - dir.c: refactor is_excluded()\n - dir.c: refactor is_excluded_from_list()\n - dir.c: rename excluded() to is_excluded()\n - dir.c: rename excluded_from_list() to is_excluded_from_list()\n - dir.c: rename path_excluded() to is_path_excluded()\n - dir.c: rename cryptic 'which' variable to more consistent name\n\n Duy helped to reroll this.\n\n Expecting a re-roll.\n\n\n* so/prompt-command (2012-10-17) 4 commits\n  (merged to 'next' on 2012-10-25 at 79565a1)\n + coloured git-prompt: paint detached HEAD marker in red\n + Fix up colored git-prompt\n + show color hints based on state of the git tree\n + Allow __git_ps1 to be used in PROMPT_COMMAND\n\n Updates __git_ps1 so that it can be used as $PROMPT_COMMAND,\n instead of being used for command substitution in $PS1, to embed\n color escape sequences in its output.\n\n Will cook in 'next'.\n\n\n* aw/rebase-am-failure-detection (2012-10-11) 1 commit\n - rebase: Handle cases where format-patch fails\n\n I am unhappy a bit about the possible performance implications of\n having to store the output in a temporary file only for a rare case\n of format-patch aborting.\n\n\n* nd/wildmatch (2012-10-15) 13 commits\n  (merged to 'next' on 2012-10-25 at 510e8df)\n + Support \"**\" wildcard in .gitignore and .gitattributes\n + wildmatch: make /**/ match zero or more directories\n + wildmatch: adjust \"**\" behavior\n + wildmatch: fix case-insensitive matching\n + wildmatch: remove static variable force_lower_case\n + wildmatch: make wildmatch's return value compatible with fnmatch\n + t3070: disable unreliable fnmatch tests\n + Integrate wildmatch to git\n + wildmatch: follow Git's coding convention\n + wildmatch: remove unnecessary functions\n + Import wildmatch from rsync\n + ctype: support iscntrl, ispunct, isxdigit and isprint\n + ctype: make sane_ctype[] const array\n\n Allows pathname patterns in .gitignore and .gitattributes files\n with double-asterisks \"foo/**/bar\" to match any number of directory\n hierarchies.\n\n I suspect that this needs to be plugged to pathspec matching code;\n otherwise \"git log -- 'Docum*/**/*.txt'\" would not show the log for\n commits that touch Documentation/git.txt, which would be confusing\n to the users.\n\n Will cook in 'next'.\n\n\n* jk/lua-hackery (2012-10-07) 6 commits\n - pretty: fix up one-off format_commit_message calls\n - Minimum compilation fixup\n - Makefile: make \"lua\" a bit more configurable\n - add a \"lua\" pretty format\n - add basic lua infrastructure\n - pretty: make some commit-parsing helpers more public\n\n Interesting exercise. When we do this for real, we probably would want\n to wrap a commit to make it more like an \"object\" with methods like\n \"parents\", etc.\n\n\n* nd/pretty-placeholder-with-color-option (2012-09-30) 9 commits\n . pretty: support %>> that steal trailing spaces\n . pretty: support truncating in %>, %< and %><\n . pretty: support padding placeholders, %< %> and %><\n . pretty: two phase conversion for non utf-8 commits\n . utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences\n . utf8.c: move display_mode_esc_sequence_len() for use by other functions\n . pretty: support %C(auto[,N]) to turn on coloring on next placeholder(s)\n . pretty: split parsing %C into a separate function\n . pretty: share code between format_decoration and show_decorations\n\n This causes warnings with -Wuninitialized, so I've ejected it from pu\n for the time being.\n\n\n* jc/maint-fetch-tighten-refname-check (2012-10-19) 1 commit\n  (merged to 'next' on 2012-11-04 at eda85ef)\n + get_fetch_map(): tighten checks on dest refs\n\n This was split out from discarded jc/maint-push-refs-all topic.\n\n Will merge to 'master' in the fifth batch.\n\n\n* jh/symbolic-ref-d (2012-10-21) 1 commit\n  (merged to 'next' on 2012-11-04 at b0762f5)\n + git symbolic-ref --delete $symref\n\n Add \"symbolic-ref -d SYM\" to delete a symbolic ref SYM.\n\n It is already possible to remove a symbolic ref with \"update-ref -d\n --no-deref\", but it may be a good addition for completeness.\n\n Will merge to 'master' in the fifth batch.\n\n\n* jh/update-ref-d-through-symref (2012-10-21) 2 commits\n - Fix failure to delete a packed ref through a symref\n - t1400-update-ref: Add test verifying bug with symrefs in delete_ref()\n\n \"update-ref -d --deref SYM\" to delete a ref through a symbolic ref\n that points to it did not remove it correctly.\n\n\n* jk/config-ignore-duplicates (2012-10-29) 9 commits\n  (merged to 'next' on 2012-10-29 at 67fa0a2)\n + builtin/config.c: Fix a sparse warning\n  (merged to 'next' on 2012-10-25 at 233df08)\n + git-config: use git_config_with_options\n + git-config: do not complain about duplicate entries\n + git-config: collect values instead of immediately printing\n + git-config: fix regexp memory leaks on error conditions\n + git-config: remove memory leak of key regexp\n + t1300: test \"git config --get-all\" more thoroughly\n + t1300: remove redundant test\n + t1300: style updates\n\n Drop duplicate detection from git-config; this lets it\n better match the internal config callbacks, which clears up\n some corner cases with includes.\n\n Will cook in 'next'.\n\n\n* ph/submodule-sync-recursive (2012-10-29) 2 commits\n  (merged to 'next' on 2012-11-04 at a000f78)\n + Add tests for submodule sync --recursive\n + Teach --recursive to submodule sync\n\n Adds \"--recursive\" option to submodule sync.\n\n Will merge to 'master' in the fifth batch.\n\n\n* fc/completion-test-simplification (2012-10-29) 2 commits\n - completion: simplify __gitcomp test helper\n - completion: refactor __gitcomp related tests\n\n Clean up completion tests.\n\n There were some comments on the list.\n\n Expecting a re-roll.\n\n\n* fc/remote-testgit-feature-done (2012-10-29) 1 commit\n - remote-testgit: properly check for errors\n\n Needs review.\n\n\n* jk/maint-diff-grep-textconv (2012-10-28) 1 commit\n  (merged to 'next' on 2012-11-04 at 790337b)\n + diff_grep: use textconv buffers for add/deleted files\n (this branch is used by jk/pickaxe-textconv.)\n\n Fixes inconsistent use of textconv with \"git log -G\".\n\n Will merge to 'master' in the fifth batch.\n\n\n* jk/pickaxe-textconv (2012-10-28) 2 commits\n - pickaxe: use textconv for -S counting\n - pickaxe: hoist empty needle check\n (this branch uses jk/maint-diff-grep-textconv.)\n\n Use textconv filters when searching with \"log -S\".\n\n Waiting for a sanity check and review from Junio.\n\n\n* as/maint-doc-fix-no-post-rewrite (2012-11-02) 1 commit\n  (merged to 'next' on 2012-11-09 at 117a91e)\n + commit: fixup misplacement of --no-post-rewrite description\n\n Will merge to 'master' in the fifth batch.\n\n\n* fc/remote-bzr (2012-11-08) 5 commits\n - remote-bzr: update working tree\n - remote-bzr: add support for remote repositories\n - remote-bzr: add support for pushing\n - remote-bzr: add simple tests\n - Add new remote-bzr transport helper\n\n New remote helper for bzr.\n\n Will merge to 'next'.\n\n\n* fc/remote-hg (2012-11-04) 16 commits\n - remote-hg: the author email can be null\n - remote-hg: add option to not track branches\n - remote-hg: add extra author test\n - remote-hg: add tests to compare with hg-git\n - remote-hg: add bidirectional tests\n - test-lib: avoid full path to store test results\n - remote-hg: add basic tests\n - remote-hg: fake bookmark when there's none\n - remote-hg: add compat for hg-git author fixes\n - remote-hg: add support for hg-git compat mode\n - remote-hg: match hg merge behavior\n - remote-hg: make sure the encoding is correct\n - remote-hg: add support to push URLs\n - remote-hg: add support for remote pushing\n - remote-hg: add support for pushing\n - Add new remote-hg transport helper\n\n New remote helper for hg.\n\n Will merge to 'next'.\n\n\n* jk/maint-http-half-auth-fetch (2012-10-31) 2 commits\n  (merged to 'next' on 2012-11-09 at af69926)\n + remote-curl: retry failed requests for auth even with gzip\n + remote-curl: hoist gzip buffer size to top of post_rpc\n\n Fixes fetch from servers that ask for auth only during the actual\n packing phase. This is not really a recommended configuration, but it\n cleans up the code at the same time.\n\n Will merge to 'master' in the sixth batch.\n\n\n* js/hp-nonstop (2012-10-30) 1 commit\n  (merged to 'next' on 2012-11-09 at fe58205)\n + fix 'make test' for HP NonStop\n\n Will merge to 'master' in the fifth batch.\n\n\n* kb/preload-index-more (2012-11-02) 1 commit\n  (merged to 'next' on 2012-11-09 at a750ebd)\n + update-index/diff-index: use core.preloadindex to improve performance\n\n Use preloadindex in more places, which has a nice speedup on systems\n with slow stat calls (and even on Linux).\n\n Will merge to 'master' in the sixth batch.\n\n\n* mh/notes-string-list (2012-11-08) 5 commits\n  (merged to 'next' on 2012-11-09 at 7a4c58c)\n + string_list_add_refs_from_colon_sep(): use string_list_split()\n + notes: fix handling of colon-separated values\n + combine_notes_cat_sort_uniq(): sort and dedup lines all at once\n + Initialize sort_uniq_list using named constant\n + string_list: add a function string_list_remove_empty_items()\n\n Improve the asymptotic performance of the cat_sort_uniq notes merge\n strategy.\n\n Will merge to 'master' in the fifth batch.\n\n\n* mh/strbuf-split (2012-11-04) 4 commits\n  (merged to 'next' on 2012-11-09 at fa984b1)\n + strbuf_split*(): document functions\n + strbuf_split*(): rename \"delim\" parameter to \"terminator\"\n + strbuf_split_buf(): simplify iteration\n + strbuf_split_buf(): use ALLOC_GROW()\n\n Cleanups and documentation for strbuf_split.\n\n Will merge to 'master' in the fifth batch.\n\n\n* mm/maint-doc-commit-edit (2012-11-02) 1 commit\n  (merged to 'next' on 2012-11-09 at 8dab7f5)\n + Document 'git commit --no-edit' explicitly\n\n Will merge to 'master' in the fifth batch.\n"},{"id":"202721","messageId":"CAN0XMO+sgSgkqK7D6=5NL--rMYBwsGohgQ7v+ja4Twub20N1sQ@mail.gmail.com","threadId":"32049","inReplyTo":"20121109192336.GA9401@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2012-11-09T20:01:00Z","receivedAt":"2012-11-09T20:01:00Z","isPatch":false,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"On Fri, Nov 9, 2012 at 8:23 PM, Jeff King <peff@peff.net> wrote:\n>\n> You can find the changes described here in the integration branches of\n> my repository at:\n>\n>   git://github.com/peff/git.git\n\nIt seems that the repo doesn't contain the integration branches?!?\n\n$ git remote add peff git://github.com/peff/git.git\n$ git fetch -v peff\nFrom git://github.com/peff/git\n * [new branch]      maint      -> peff/maint\n * [new branch]      master     -> peff/master\n * [new branch]      next       -> peff/next\n * [new branch]      pu         -> peff/pu\n * [new branch]      todo       -> peff/todo\n$\n"},{"id":"202722","messageId":"CAN0XMOL1SS2iiJkZyt3HACoCRXE1nGrd52B2rscWyS1e7X0GWQ@mail.gmail.com","threadId":"32049","inReplyTo":"CAN0XMO+sgSgkqK7D6=5NL--rMYBwsGohgQ7v+ja4Twub20N1sQ@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2012-11-09T20:06:47Z","receivedAt":"2012-11-09T20:06:47Z","isPatch":false,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"On Fri, Nov 9, 2012 at 9:01 PM, Ralf Thielow <ralf.thielow@gmail.com> wrote:\n> On Fri, Nov 9, 2012 at 8:23 PM, Jeff King <peff@peff.net> wrote:\n>>\n>> You can find the changes described here in the integration branches of\n>> my repository at:\n>>\n>>   git://github.com/peff/git.git\n>\n> It seems that the repo doesn't contain the integration branches?!?\n>\n> $ git remote add peff git://github.com/peff/git.git\n> $ git fetch -v peff\n> From git://github.com/peff/git\n>  * [new branch]      maint      -> peff/maint\n>  * [new branch]      master     -> peff/master\n>  * [new branch]      next       -> peff/next\n>  * [new branch]      pu         -> peff/pu\n>  * [new branch]      todo       -> peff/todo\n> $\n\nBut \"integration branches\" means \"master\", \"next\" and \"pu\" than I haven't\nsaid anything. ;) Sorry for the noise.\n"},{"id":"202723","messageId":"20121109201057.GA11368@sigill.intra.peff.net","threadId":"32049","inReplyTo":"CAN0XMOL1SS2iiJkZyt3HACoCRXE1nGrd52B2rscWyS1e7X0GWQ@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-09T20:10:57Z","receivedAt":"2012-11-09T20:10:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 09, 2012 at 09:06:47PM +0100, Ralf Thielow wrote:\n\n> > It seems that the repo doesn't contain the integration branches?!?\n> >\n> > $ git remote add peff git://github.com/peff/git.git\n> > $ git fetch -v peff\n> > From git://github.com/peff/git\n> >  * [new branch]      maint      -> peff/maint\n> >  * [new branch]      master     -> peff/master\n> >  * [new branch]      next       -> peff/next\n> >  * [new branch]      pu         -> peff/pu\n> >  * [new branch]      todo       -> peff/todo\n> > $\n> \n> But \"integration branches\" means \"master\", \"next\" and \"pu\" than I haven't\n> said anything. ;) Sorry for the noise.\n\nRight. :)\n\nI have not been pushing the individual topic branches to make life\neasier for people who usually just track Junio's kernel.org repository,\nand would not welcome suddenly getting a hundred extra remote branches.\nI can make them public if it makes life easier for people, but it may\nnot be worth it at this point, with Junio returning soon.\n\n-Peff\n"},{"id":"202724","messageId":"7vr4o2plmw.fsf@alter.siamese.dyndns.org","threadId":"32049","inReplyTo":"20121109201057.GA11368@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-09T20:27:35Z","receivedAt":"2012-11-09T20:27:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I have not been pushing the individual topic branches to make life\n> easier for people who usually just track Junio's kernel.org repository,\n> and would not welcome suddenly getting a hundred extra remote branches.\n> I can make them public if it makes life easier for people, but it may\n> not be worth it at this point, with Junio returning soon.\n\nWhat we should have arranged was to have https://github.com/git/git\n(which is not even owned by me, but I asked somebody at GitHub to\nassign me a write privilege) writable by the interim maintainer, so\nthat normal people would keep pulling from there, while the interim\nmaintainer can choose to publish broken-out branches to his\nrepository.\n\nAnd it is not too late to do so; from the look of your \"What's\ncooking\", you are doing pretty well ;-).\n"},{"id":"202728","messageId":"87a9uq5tql.fsf@Niukka.kon.iki.fi","threadId":"32049","inReplyTo":"20121109192336.GA9401@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Kalle Olavi Niemitalo","fromEmail":"kon@iki.fi","sentAt":"2012-11-09T21:52:50Z","receivedAt":"2012-11-09T21:52:50Z","isPatch":false,"sender":{"key":"kon@iki.fi","avatar":null},"body":"Jeff King <peff@peff.net> writes:\n\n>  Comments welcome from people using unusual editors (e.g., a script that\n>  starts an editor in another window then blocks, waiting for the user to\n>  finish).\n\nI often run a shell in Emacs in X, then start git commit in that\nshell.  $EDITOR is emacsclient --current-frame, which asks the\nexisting Emacs instance to load the file and waits until I press\nC-x # in Emacs to mark the file done.  If I want to abort the\ncommit, it is most intuitive to return to the *Shell* buffer in\nEmacs and press C-c C-c (comint-interrupt-subjob) to send SIGINT\nto git from there.  (I see that \"an empty message aborts the\ncommit\", and indeed it does, but well, I prefer not to trust such\na feature if I can instead just interrupt the thing.)\n\nWith pf/editor-ignore-sigint, C-c C-c in the *Shell* buffer kills\nneither git nor the emacsclient started by git.  This is not good.\nSIGQUIT from C-c C-\\ (comint-quit-subjob) still works though.\n"},{"id":"202729","messageId":"CAMP44s3yVtQ4wGqVTyHN-VfAM7iRo9WfNnAu+ns7Zkc_cPBH3g@mail.gmail.com","threadId":"32049","inReplyTo":"20121109192336.GA9401@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-09T23:21:48Z","receivedAt":"2012-11-09T23:21:48Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nOn Fri, Nov 9, 2012 at 8:23 PM, Jeff King <peff@peff.net> wrote:\n\n> * fc/fast-export-fixes (2012-11-08) 14 commits\n>  - fast-export: don't handle uninteresting refs\n>  - fast-export: make sure updated refs get updated\n>  - fast-export: fix comparison in tests\n>  - fast-export: trivial cleanup\n>  - remote-testgit: make clear the 'done' feature\n>  - remote-testgit: report success after an import\n>  - remote-testgit: exercise more features\n>  - remote-testgit: cleanup tests\n>  - remote-testgit: remove irrelevant test\n>  - remote-testgit: get rid of non-local functionality\n>  - Add new simplified git-remote-testgit\n>  - Rename git-remote-testgit to git-remote-testpy\n>  - remote-testgit: fix direction of marks\n>  - fast-export: avoid importing blob marks\n>\n>  Improvements to fix fast-export bugs, including how refs pointing to\n>  already-seen commits are handled. An earlier 4-commit version of this\n>  series looked good to me, but this much-expanded version has not seen\n>  any comments.\n>\n>  Needs review.\n\nI can send the previous 4-commit version if needed, the only thing\nthat changed is the commit messages.\n\nI think it's unfortunate that 4-commit version would not be mentioning\nthat it fixes the above tests, but hey; I did what I could.\n\n> * fc/zsh-completion (2012-10-29) 3 commits\n>  - completion: add new zsh completion\n>  - completion: add new __gitcompadd helper\n>  - completion: get rid of empty COMPREPLY assignments\n>\n>  There were some comments on this, but I wasn't clear on the outcome.\n>\n>  Need to take a closer look.\n\nSZEDER should probably take a look. I think it should be better than\nthe previous series.\n\n> * fc/completion-test-simplification (2012-10-29) 2 commits\n>  - completion: simplify __gitcomp test helper\n>  - completion: refactor __gitcomp related tests\n>\n>  Clean up completion tests.\n>\n>  There were some comments on the list.\n>\n>  Expecting a re-roll.\n\nThe second patch I can re-roll, but the first patch needs some\nexternal input. My preference is that tests should also be simple and\nmaintainable, SZEDER's preference is that tests are better being\nexplicit and verbose (even if harder to maintain) to minimize possible\nissues in the tests.\n\n> * fc/remote-testgit-feature-done (2012-10-29) 1 commit\n>  - remote-testgit: properly check for errors\n>\n>  Needs review.\n\nSverre probably should reply. I think I already addressed his comments\nand the patch should be OK to push.\n\nBut probably it's not that important considering the testgit\nrefactoring, and also I'm thinking that we need to actually check the\nstatus of the process[1] because the situation is still not OK with\npushing, and I'm learning it the hard way with a buggy remote helper.\n\n> * fc/remote-bzr (2012-11-08) 5 commits\n>  - remote-bzr: update working tree\n>  - remote-bzr: add support for remote repositories\n>  - remote-bzr: add support for pushing\n>  - remote-bzr: add simple tests\n>  - Add new remote-bzr transport helper\n>\n>  New remote helper for bzr.\n>\n>  Will merge to 'next'.\n\nI already have a newer version of this with support for special modes:\nexecutable files, symlinks, etc. I think a reroll would make sense.\n\n> * fc/remote-hg (2012-11-04) 16 commits\n>  - remote-hg: the author email can be null\n>  - remote-hg: add option to not track branches\n>  - remote-hg: add extra author test\n>  - remote-hg: add tests to compare with hg-git\n>  - remote-hg: add bidirectional tests\n>  - test-lib: avoid full path to store test results\n>  - remote-hg: add basic tests\n>  - remote-hg: fake bookmark when there's none\n>  - remote-hg: add compat for hg-git author fixes\n>  - remote-hg: add support for hg-git compat mode\n>  - remote-hg: match hg merge behavior\n>  - remote-hg: make sure the encoding is correct\n>  - remote-hg: add support to push URLs\n>  - remote-hg: add support for remote pushing\n>  - remote-hg: add support for pushing\n>  - Add new remote-hg transport helper\n>\n>  New remote helper for hg.\n>\n>  Will merge to 'next'.\n\n:)\n\nI have a few patches on top of this, but they can probably wait.\n\nCheers.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/208139\n\n-- \nFelipe Contreras\n"},{"id":"202730","messageId":"20121110003331.GA12567@sigill.intra.peff.net","threadId":"32049","inReplyTo":"CAMP44s3yVtQ4wGqVTyHN-VfAM7iRo9WfNnAu+ns7Zkc_cPBH3g@mail.gmail.com","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-10T00:33:31Z","receivedAt":"2012-11-10T00:33:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:\n\n> > * fc/fast-export-fixes (2012-11-08) 14 commits\n> >  - fast-export: don't handle uninteresting refs\n> >  - fast-export: make sure updated refs get updated\n> >  - fast-export: fix comparison in tests\n> >  - fast-export: trivial cleanup\n> >  - remote-testgit: make clear the 'done' feature\n> >  - remote-testgit: report success after an import\n> >  - remote-testgit: exercise more features\n> >  - remote-testgit: cleanup tests\n> >  - remote-testgit: remove irrelevant test\n> >  - remote-testgit: get rid of non-local functionality\n> >  - Add new simplified git-remote-testgit\n> >  - Rename git-remote-testgit to git-remote-testpy\n> >  - remote-testgit: fix direction of marks\n> >  - fast-export: avoid importing blob marks\n> >\n> >  Improvements to fix fast-export bugs, including how refs pointing to\n> >  already-seen commits are handled. An earlier 4-commit version of this\n> >  series looked good to me, but this much-expanded version has not seen\n> >  any comments.\n> >\n> >  Needs review.\n> \n> I can send the previous 4-commit version if needed, the only thing\n> that changed is the commit messages.\n\nIn the actual code, perhaps, but aren't there significant changes to the\ngit-remote-testgit infrastructure that were not originally present? That\ncould use some review.\n\nI also seem to recall that the tests in this version rely on the presence of bash;\ndon't we still need to mark the tests with a prerequisite?\n\n> > * fc/completion-test-simplification (2012-10-29) 2 commits\n> >  - completion: simplify __gitcomp test helper\n> >  - completion: refactor __gitcomp related tests\n> >\n> >  Clean up completion tests.\n> >\n> >  There were some comments on the list.\n> >\n> >  Expecting a re-roll.\n> \n> The second patch I can re-roll, but the first patch needs some\n> external input. My preference is that tests should also be simple and\n> maintainable, SZEDER's preference is that tests are better being\n> explicit and verbose (even if harder to maintain) to minimize possible\n> issues in the tests.\n\nI think it is better to keep the tests simple and maintainable. If there\nare multiple ways to do things and they all need testing, then that\nshould be clear from the tests, not done haphazardly because some tests\nhappen to use a different way of doing things.\n\nI seem to recall there was a one-liner fix that needed to be rolled in,\nwhich is why I held it out of next.\n\n> > * fc/remote-bzr (2012-11-08) 5 commits\n> >  - remote-bzr: update working tree\n> >  - remote-bzr: add support for remote repositories\n> >  - remote-bzr: add support for pushing\n> >  - remote-bzr: add simple tests\n> >  - Add new remote-bzr transport helper\n> >\n> >  New remote helper for bzr.\n> >\n> >  Will merge to 'next'.\n> \n> I already have a newer version of this with support for special modes:\n> executable files, symlinks, etc. I think a reroll would make sense.\n\nThanks for letting me know.\n\n-Peff\n"},{"id":"202731","messageId":"20121110003850.GB12567@sigill.intra.peff.net","threadId":"32049","inReplyTo":"7vr4o2plmw.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-10T00:38:50Z","receivedAt":"2012-11-10T00:38:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 09, 2012 at 12:27:35PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I have not been pushing the individual topic branches to make life\n> > easier for people who usually just track Junio's kernel.org repository,\n> > and would not welcome suddenly getting a hundred extra remote branches.\n> > I can make them public if it makes life easier for people, but it may\n> > not be worth it at this point, with Junio returning soon.\n> \n> What we should have arranged was to have https://github.com/git/git\n> (which is not even owned by me, but I asked somebody at GitHub to\n> assign me a write privilege) writable by the interim maintainer, so\n> that normal people would keep pulling from there, while the interim\n> maintainer can choose to publish broken-out branches to his\n> repository.\n\nYes, I have write access to that repository, too, but I intentionally\nheld off from updating it out of a sense of nervousness. I figured if I\nscrewed up anything too badly, people who were clued-in enough to switch\nto pulling from my repository would be clued-in enough to rebase across\nany too-horrible mistake I made. ;)\n\nI think if we do this again, I will make the same split you do (git/git\nfor integration branches, peff/git as a mirror of my private repo).\n\n> And it is not too late to do so; from the look of your \"What's\n> cooking\", you are doing pretty well ;-).\n\nAny fool can merge topics to master. The real test will be how many\nregressions people report in the next two weeks. :)\n\nBy the way, I did not touch 'maint' at all while you were gone. I don't\nknow what your usual method is for keeping track of maint-worthy topics\nafter they have gone to master. The usual \"what's cooking\" workflow\nkeeps track of things going to master, but no more; I'd guess you\nprobably just merge to maint when you delete them from last cycle's\n\"graduated to master\" list.\n\nI just let them stew in master for a bit longer, and we can easily find\nand merge them with \"git branch --no-merged maint | grep maint\".\n\n-Peff\n"},{"id":"202732","messageId":"CAMP44s058Km_=xUn8UvwYwuk0DStTB3bMYu-HPsQWDF-pT_6EQ@mail.gmail.com","threadId":"32049","inReplyTo":"20121110003331.GA12567@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-10T00:44:20Z","receivedAt":"2012-11-10T00:44:20Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 10, 2012 at 1:33 AM, Jeff King <peff@peff.net> wrote:\n> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:\n>\n>> > * fc/fast-export-fixes (2012-11-08) 14 commits\n>> >  - fast-export: don't handle uninteresting refs\n>> >  - fast-export: make sure updated refs get updated\n>> >  - fast-export: fix comparison in tests\n>> >  - fast-export: trivial cleanup\n>> >  - remote-testgit: make clear the 'done' feature\n>> >  - remote-testgit: report success after an import\n>> >  - remote-testgit: exercise more features\n>> >  - remote-testgit: cleanup tests\n>> >  - remote-testgit: remove irrelevant test\n>> >  - remote-testgit: get rid of non-local functionality\n>> >  - Add new simplified git-remote-testgit\n>> >  - Rename git-remote-testgit to git-remote-testpy\n>> >  - remote-testgit: fix direction of marks\n>> >  - fast-export: avoid importing blob marks\n>> >\n>> >  Improvements to fix fast-export bugs, including how refs pointing to\n>> >  already-seen commits are handled. An earlier 4-commit version of this\n>> >  series looked good to me, but this much-expanded version has not seen\n>> >  any comments.\n>> >\n>> >  Needs review.\n>>\n>> I can send the previous 4-commit version if needed, the only thing\n>> that changed is the commit messages.\n>\n> In the actual code, perhaps, but aren't there significant changes to the\n> git-remote-testgit infrastructure that were not originally present? That\n> could use some review.\n>\n> I also seem to recall that the tests in this version rely on the presence of bash;\n> don't we still need to mark the tests with a prerequisite?\n\nI meant in the 4-commits.\n\n>> > * fc/completion-test-simplification (2012-10-29) 2 commits\n>> >  - completion: simplify __gitcomp test helper\n>> >  - completion: refactor __gitcomp related tests\n>> >\n>> >  Clean up completion tests.\n>> >\n>> >  There were some comments on the list.\n>> >\n>> >  Expecting a re-roll.\n>>\n>> The second patch I can re-roll, but the first patch needs some\n>> external input. My preference is that tests should also be simple and\n>> maintainable, SZEDER's preference is that tests are better being\n>> explicit and verbose (even if harder to maintain) to minimize possible\n>> issues in the tests.\n>\n> I think it is better to keep the tests simple and maintainable. If there\n> are multiple ways to do things and they all need testing, then that\n> should be clear from the tests, not done haphazardly because some tests\n> happen to use a different way of doing things.\n\nGood, that's what my first patch does; no functional changes, just\nrefactor code into a single function.\n\n> I seem to recall there was a one-liner fix that needed to be rolled in,\n> which is why I held it out of next.\n\nYes, that I can reroll.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202750","messageId":"20121110123250.GR12052@goldbirke","threadId":"32049","inReplyTo":"20121110003331.GA12567@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2012-11-10T12:32:50Z","receivedAt":"2012-11-10T12:32:50Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\nOn Fri, Nov 09, 2012 at 07:33:31PM -0500, Jeff King wrote:\n> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:\n> > > * fc/completion-test-simplification (2012-10-29) 2 commits\n> > >  - completion: simplify __gitcomp test helper\n> > >  - completion: refactor __gitcomp related tests\n> > >\n> > >  Clean up completion tests.\n> > >\n> > >  There were some comments on the list.\n> > >\n> > >  Expecting a re-roll.\n> > \n> > The second patch I can re-roll, but the first patch needs some\n> > external input. My preference is that tests should also be simple and\n> > maintainable, SZEDER's preference is that tests are better being\n> > explicit and verbose (even if harder to maintain) to minimize possible\n> > issues in the tests.\n> \n> I think it is better to keep the tests simple and maintainable.\n\nMaintainable?  There is nothing to maintain here.  Users' completion\nscripts depend on __gitcomp(), so its behavior shouldn't be changed.\nIt can only be extended by a fifth parameter or by quoting words when\nnecessary, but these future changes must not alter the current\nbehavior checked by these tests, therefore even then these tests must\nbe left intact.\n\nSimple?  Currently you only need to look at __gitcomp() and the test\nitself to understand what's going on.  With this series you'll also\nneed to look at test_gitcomp(), figure out what its parameters are\nsupposed to mean, and possibly get puzzled on the way why __gitcomp()\nis now seemingly called with only one parameter.\n\nSo, I don't see much benefit in this series (except the part to use\nprint_comp instead of \"change IFS && echo\", but that's already done in\nthis patch:\nhttp://article.gmane.org/gmane.comp.version-control.git/207927).\n\nOTOH, this series has some serious drawbacks.\n\nIt makes debugging more difficult.  While working on the quoting\nissues I managed to break completion tests many-many times lately.  In\nnormal tests I could add a few debugging instructions to the failed\ntest to find out where the breakage lies, without affecting other\ntests.  However, if the failed test uses the test_completion() helper,\nthen I have to add debugging instructions to test_completion() itself,\ntoo.  This is bad, because many tests use this helper function and are\ntherefore affected by the debugging instructions, producing truckloads\nof output making it difficult to dig out the relevant parts, or, worse\nyet, causing breakages in other tests.  With this series the same\ndifficulties will come to __gitcomp() tests, too.\n\nIt can also encourage writing bad tests, similar to those that managed\nto cram many test_completion() lines into a single tests, giving me\nheadaches to figure out what went wrong this time.\n\n\nBest,\nGábor\n"},{"id":"202758","messageId":"20121110155209.75EFC2E8B68@grass.foxharp.boston.ma.us","threadId":"32049","inReplyTo":"87a9uq5tql.fsf@Niukka.kon.iki.fi","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Paul Fox","fromEmail":"pgf@foxharp.boston.ma.us","sentAt":"2012-11-10T15:52:09Z","receivedAt":"2012-11-10T15:52:09Z","isPatch":false,"sender":{"key":"pgf@foxharp.boston.ma.us","avatar":"https://avatars.githubusercontent.com/u/4249842?v=4"},"body":"kalle olavi niemitalo wrote:\n > Jeff King <peff@peff.net> writes:\n > \n > >  Comments welcome from people using unusual editors (e.g., a script that\n > >  starts an editor in another window then blocks, waiting for the user to\n > >  finish).\n > \n > I often run a shell in Emacs in X, then start git commit in that\n > shell.  $EDITOR is emacsclient --current-frame, which asks the\n > existing Emacs instance to load the file and waits until I press\n > C-x # in Emacs to mark the file done.  If I want to abort the\n > commit, it is most intuitive to return to the *Shell* buffer in\n > Emacs and press C-c C-c (comint-interrupt-subjob) to send SIGINT\n > to git from there.  (I see that \"an empty message aborts the\n > commit\", and indeed it does, but well, I prefer not to trust such\n > a feature if I can instead just interrupt the thing.)\n > \n > With pf/editor-ignore-sigint, C-c C-c in the *Shell* buffer kills\n > neither git nor the emacsclient started by git.  This is not good.\n > SIGQUIT from C-c C-\\ (comint-quit-subjob) still works though.\n\nwhen i implemented the change, i wondered if some twisted emacs\nworkflow would be an issue. ;-)  and i almost blocked SIGQUIT as\nwell -- the two programs i looked at for precedent (CVS and MH) both\nblock both SIGQUIT and SIGINT when spawning an editor.\n\nbut since emacs users must have dealt with CVS for a long time before\ndealing with git, how might they have done so?\n\nthe existing git behavior is bad for non-emacs users, and git itself\nprovides an abort-the-operation mechanism (i.e., writing an empty\nmessage), so i'm not convinced your use case invalidates the new\nbehavior.  (though it might spotlight a need for this being prominent\nin release notes.)\n\npaul\n=---------------------\n paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 40.6 degrees)\n"},{"id":"202761","messageId":"7vhaoxpehx.fsf@alter.siamese.dyndns.org","threadId":"32049","inReplyTo":"20121110003850.GB12567@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-10T17:14:02Z","receivedAt":"2012-11-10T17:14:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Nov 09, 2012 at 12:27:35PM -0800, Junio C Hamano wrote:\n>\n>> What we should have arranged was to have https://github.com/git/git\n>> (which is not even owned by me, but I asked somebody at GitHub to\n>> assign me a write privilege) writable by the interim maintainer, so\n>> that normal people would keep pulling from there, while the interim\n>> maintainer can choose to publish broken-out branches to his\n>> repository.\n>\n> Yes, I have write access to that repository, too, but I intentionally\n> held off from updating it out of a sense of nervousness. I figured if I\n> screwed up anything too badly, people who were clued-in enough to switch\n> to pulling from my repository would be clued-in enough to rebase across\n> any too-horrible mistake I made. ;)\n\nThat \"nervousness\" reminds me of myself when I took over.  Before I\ncould ask for a few weeks of practice period, Linus arranged to have\nfolks at k.org to chown the authoritative location to me, declaring\n\"no practice period; it's already done and it's all yours\".\n\nAnd I made at least one mistake pushing 'master' with one commit\nrewound too much (corrected by pushing an extra merge).  Luckily,\nthe world did not end ;-).\n\n> I think if we do this again, I will make the same split you do (git/git\n> for integration branches, peff/git as a mirror of my private repo).\n\nI am fairly sure I'll have to ask you (or somebody else) again next\nyear around late September.\n\n>> And it is not too late to do so; from the look of your \"What's\n>> cooking\", you are doing pretty well ;-).\n>\n> Any fool can merge topics to master. The real test will be how many\n> regressions people report in the next two weeks. :)\n\nI agree that the actual merging to 'master' is mechanical with the\nprocedure built around Meta/Reintegrate.  Important decisions are\nmade before you merge a topic to 'next' and mark topics as \"Will\nmerge to 'master'.\"  My comment was about that, and your responses\nto the list messages.\n\n> By the way, I did not touch 'maint' at all while you were gone. I don't\n> know what your usual method is for keeping track of maint-worthy topics\n> after they have gone to master. The usual \"what's cooking\" workflow\n> keeps track of things going to master, but no more; I'd guess you\n> probably just merge to maint when you delete them from last cycle's\n> \"graduated to master\" list.\n\nThat is done by eyeballing output from Meta/GRADUATED (which spits\nout something that could be fed to shell, but I do not fully trust\nits logic, and always eyeball them before I prepare the temporary\nfile to feed Meta/Reintegrate to update 'maint').\n"},{"id":"202770","messageId":"CAMP44s2WHZh8Jc6PaM-9h9Hkf3Un_xAC=29TNFF1u9sSEYA7gg@mail.gmail.com","threadId":"32049","inReplyTo":"20121110123250.GR12052@goldbirke","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-10T19:13:37Z","receivedAt":"2012-11-10T19:13:37Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Nov 10, 2012 at 1:32 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> Hi,\n>\n> On Fri, Nov 09, 2012 at 07:33:31PM -0500, Jeff King wrote:\n>> On Sat, Nov 10, 2012 at 12:21:48AM +0100, Felipe Contreras wrote:\n>> > > * fc/completion-test-simplification (2012-10-29) 2 commits\n>> > >  - completion: simplify __gitcomp test helper\n>> > >  - completion: refactor __gitcomp related tests\n>> > >\n>> > >  Clean up completion tests.\n>> > >\n>> > >  There were some comments on the list.\n>> > >\n>> > >  Expecting a re-roll.\n>> >\n>> > The second patch I can re-roll, but the first patch needs some\n>> > external input. My preference is that tests should also be simple and\n>> > maintainable, SZEDER's preference is that tests are better being\n>> > explicit and verbose (even if harder to maintain) to minimize possible\n>> > issues in the tests.\n>>\n>> I think it is better to keep the tests simple and maintainable.\n>\n> Maintainable?  There is nothing to maintain here.  Users' completion\n> scripts depend on __gitcomp(), so its behavior shouldn't be changed.\n> It can only be extended by a fifth parameter or by quoting words when\n> necessary, but these future changes must not alter the current\n> behavior checked by these tests, therefore even then these tests must\n> be left intact.\n\nI disagree. If we add a new parameter to __gitcomp(), and we need to\nadd a new parameter to test_gitcomp(), so be it. Yes, we might change\nthe behavior of the other tests, but that's what reviews are for: to\nmake sure we don't alter other behavior by mistake. That's what we do\nfor code, and that what we should do for tests.\n\nBut in this particular case nothing would need to change because\ntest_gitcomp() would pass whatever arguments it receives, being four,\nor five, or twenty. So this is not a concern, maybe some other kind of\nchange, but not this.\n\nCompare this:\nhttp://article.gmane.org/gmane.comp.version-control.git/208168\n\nTo this:\nhttp://article.gmane.org/gmane.comp.version-control.git/207927\n\nIf we ever need to make changes to the __gitcomp tests, a small change\nis better than a big change; IOW: the test code would be more\nmaintainable.\n\nEven more, the end result is much less code: less code = more maintainability.\n\n> Simple?  Currently you only need to look at __gitcomp() and the test\n> itself to understand what's going on.  With this series you'll also\n> need to look at test_gitcomp(), figure out what its parameters are\n> supposed to mean, and possibly get puzzled on the way why __gitcomp()\n> is now seemingly called with only one parameter.\n\nMaybe it's easier for you to understand, but certainly not for other\npeople: 'declare -a COMPREPLY'? cur? print_comp? What does that even\nmean? Chances are most people don't even know what __gitcomp is.\nFortunately they don't have to dig too deep to find all those things\nout, but they can do the same for test_gitcomp().\n\nIt might make sense to add some comment on top of test_gitcomp to\nexplain the arguments and the input to make things easier, but that\nwould only make it more readable, not less, than the current\nsituation, because right now you would have to add a similar comment\nto each and every block of code that calls __gitcomp.\n\nFunctions make our life easier.\n\n> So, I don't see much benefit in this series (except the part to use\n> print_comp instead of \"change IFS && echo\", but that's already done in\n> this patch:\n> http://article.gmane.org/gmane.comp.version-control.git/207927).\n\nYes, and that version is much bigger than this:\nhttp://article.gmane.org/gmane.comp.version-control.git/208168\n\nCode that allows the later patch is more maintainable.\n\n> OTOH, this series has some serious drawbacks.\n>\n> It makes debugging more difficult.  While working on the quoting\n> issues I managed to break completion tests many-many times lately.  In\n> normal tests I could add a few debugging instructions to the failed\n> test to find out where the breakage lies, without affecting other\n> tests.  However, if the failed test uses the test_completion() helper,\n> then I have to add debugging instructions to test_completion() itself,\n> too.  This is bad, because many tests use this helper function and are\n> therefore affected by the debugging instructions, producing truckloads\n> of output making it difficult to dig out the relevant parts, or, worse\n> yet, causing breakages in other tests.  With this series the same\n> difficulties will come to __gitcomp() tests, too.\n\nWhat I do is copy the function to test_gitcomp2() and add the\ndebugging there, and only call it from the places where I want the\ndebugging. I don't think this is an issue.\n\nIn fact, I think it's an advantage; sometimes that's exactly what you\nwant, to add the debugging for everything.\n\n> It can also encourage writing bad tests, similar to those that managed\n> to cram many test_completion() lines into a single tests, giving me\n> headaches to figure out what went wrong this time.\n\nThat's policy. We can decide what each test-case contains right here,\non the mailing list. There's no need to have verbose code to slightly\nhint a policy.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"202778","messageId":"871ug15k5c.fsf@Niukka.kon.iki.fi","threadId":"32049","inReplyTo":"20121110155209.75EFC2E8B68@grass.foxharp.boston.ma.us","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Kalle Olavi Niemitalo","fromEmail":"kon@iki.fi","sentAt":"2012-11-10T19:32:15Z","receivedAt":"2012-11-10T19:32:15Z","isPatch":false,"sender":{"key":"kon@iki.fi","avatar":null},"body":"Paul Fox <pgf@foxharp.boston.ma.us> writes:\n\n> when i implemented the change, i wondered if some twisted emacs\n> workflow would be an issue. ;-)  and i almost blocked SIGQUIT as\n> well -- the two programs i looked at for precedent (CVS and MH) both\n> block both SIGQUIT and SIGINT when spawning an editor.\n>\n> but since emacs users must have dealt with CVS for a long time before\n> dealing with git, how might they have done so?\n\nI think I usually ran CVS via vc.el, which prompts for a commit\nmessage in Emacs before it runs cvs commit.  So CVS did not need\nto run $EDITOR.\n\nI just tried emacsclient with CVS 1.12.13-MirDebian-9, and it\nbehaves somewhat differently from Git with pf/editor-ignore-sigint.\nWhen I tell Emacs to send SIGINT to the *Shell* buffer, CVS prompts:\n\ncvs commit: warning: editor session failed\n\nLog message unchanged or not specified\na)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs\nAction: (continue) \n\nand then I can choose to abort.\n\nWith strace, it looks like CVS sets SIG_IGN as the handler of\nSIGINT and SIGQUIT only in the parent process after forking, not\nin the child process that executes the editor.\n\nCVS also temporarily blocks signals by calling sigprocmask, but\nit undoes that before it forks or waits for the child process.\n"},{"id":"202779","messageId":"7vd2zlp6zb.fsf@alter.siamese.dyndns.org","threadId":"32049","inReplyTo":"20121110123250.GR12052@goldbirke","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-10T19:56:24Z","receivedAt":"2012-11-10T19:56:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder@ira.uka.de> writes:\n\n>> I think it is better to keep the tests simple and maintainable.\n>\n> Maintainable?  There is nothing to maintain here....\n> ...\n> OTOH, this series has some serious drawbacks.\n>\n> It makes debugging more difficult....\n\nAre these referring to the same aspect of the series?  The concern\nyou described about debuggability matches my impression IIRC back\nwhen I took a look at the series, which I would count as a large\npart of keeping tests maintainable.\n\nBut you may be referring to something different (sorry, not on my\nprimary machine yet).\n"},{"id":"202783","messageId":"m28va9duxb.fsf@igel.home","threadId":"32049","inReplyTo":"871ug15k5c.fsf@Niukka.kon.iki.fi","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-11-10T21:12:16Z","receivedAt":"2012-11-10T21:12:16Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Kalle Olavi Niemitalo <kon@iki.fi> writes:\n\n> With strace, it looks like CVS sets SIG_IGN as the handler of\n> SIGINT and SIGQUIT only in the parent process after forking, not\n> in the child process that executes the editor.\n>\n> CVS also temporarily blocks signals by calling sigprocmask, but\n> it undoes that before it forks or waits for the child process.\n\nThis emulates what system(3) does.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"202787","messageId":"20121110220811.DC6A42E8B68@grass.foxharp.boston.ma.us","threadId":"32049","inReplyTo":"871ug15k5c.fsf@Niukka.kon.iki.fi","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Paul Fox","fromEmail":"pgf@foxharp.boston.ma.us","sentAt":"2012-11-10T22:08:11Z","receivedAt":"2012-11-10T22:08:11Z","isPatch":false,"sender":{"key":"pgf@foxharp.boston.ma.us","avatar":"https://avatars.githubusercontent.com/u/4249842?v=4"},"body":"kalle olavi niemitalo wrote:\n > Paul Fox <pgf@foxharp.boston.ma.us> writes:\n > \n > > when i implemented the change, i wondered if some twisted emacs\n > > workflow would be an issue. ;-)  and i almost blocked SIGQUIT as\n > > well -- the two programs i looked at for precedent (CVS and MH) both\n > > block both SIGQUIT and SIGINT when spawning an editor.\n > >\n > > but since emacs users must have dealt with CVS for a long time before\n > > dealing with git, how might they have done so?\n > \n > I think I usually ran CVS via vc.el, which prompts for a commit\n > message in Emacs before it runs cvs commit.  So CVS did not need\n > to run $EDITOR.\n > \n > I just tried emacsclient with CVS 1.12.13-MirDebian-9, and it\n > behaves somewhat differently from Git with pf/editor-ignore-sigint.\n > When I tell Emacs to send SIGINT to the *Shell* buffer, CVS prompts:\n > \n > cvs commit: warning: editor session failed\n > \n > Log message unchanged or not specified\n > a)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs\n > Action: (continue) \n\nyou're sending SIGINT to the cvs commit command, and that causes the\neditor to die right away?  that's surprising.  i can replicate your\ndescribed behavior by setting $VISUAL to a script that just sleeps, and\nsending SIGTERM to the cvs commit process.  but not by sending SIGINT.\n\nwell, i'm not sure what to say.  there's a real problem when using the\ncurrent code and traditional editors.  i thought that the patch in\npf/editor-ignore-sigint reflected standard practice, and indeed it\naccomplishes exactly the right thing with those editors.  you've shown\na particular work flow involving emacsclient that won't work anymore\nwith the change made, though there are workarounds.  perhaps there's\nsomething the other editors themselves should be doing differently,\nbut i don't know what that might be.\n\npaul\n\n > \n > and then I can choose to abort.\n > \n > With strace, it looks like CVS sets SIG_IGN as the handler of\n > SIGINT and SIGQUIT only in the parent process after forking, not\n > in the child process that executes the editor.\n > \n > CVS also temporarily blocks signals by calling sigprocmask, but\n > it undoes that before it forks or waits for the child process.\n\n=---------------------\n paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 45.5 degrees)\n"},{"id":"202801","messageId":"87wqxs4o6f.fsf@Niukka.kon.iki.fi","threadId":"32049","inReplyTo":"20121110220811.DC6A42E8B68@grass.foxharp.boston.ma.us","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Kalle Olavi Niemitalo","fromEmail":"kon@iki.fi","sentAt":"2012-11-11T07:02:48Z","receivedAt":"2012-11-11T07:02:48Z","isPatch":false,"sender":{"key":"kon@iki.fi","avatar":null},"body":"Paul Fox <pgf@foxharp.boston.ma.us> writes:\n\n> you're sending SIGINT to the cvs commit command, and that causes the\n> editor to die right away?\n\nThat's right.  It is not a quirk of shell-mode in Emacs, because\nI get the same result with ^C in xterm too.\n\n% EDITOR=\"$HOME/prefix/x86_64-unknown-linux-gnu/bin/emacsclient --current-frame\"\n% export EDITOR\n% cvs commit BUGIT\nWaiting for Emacs...^Ccvs commit: warning: editor session failed\n\nLog message unchanged or not specified\na)bort, c)ontinue, e)dit, !)reuse this message unchanged for remaining dirs\nAction: (continue) a\ncvs [commit aborted]: aborted by user\n% \n\nWhile cvs was waiting from emacsclient:\n\n% cat /proc/2030/stat\n2030 (cvs) S 1849 2030 1849 34816 2030 4202496 598 0 0 0 0 0 0 0 20 0 1 0 94752537 34254848 410 18446744073709551615 140168182550528 140168183348316 140737407935424 140737407931680 140168163193950 0 0 6 20513 0 0 0 17 2 0 0 0 0 0\n% grep 'Name\\|Pid\\|Sig' /proc/2030/status\nName:\tcvs\nPid:\t2030\nPPid:\t1849\nTracerPid:\t0\nSigQ:\t0/28998\nSigPnd:\t0000000000000000\nSigBlk:\t0000000000000000\nSigIgn:\t0000000000000006\nSigCgt:\t0000000180005021\n% cat /proc/2031/stat\n2031 (emacsclient) S 2030 2030 1849 34816 2030 4202496 155 0 0 0 0 0 0 0 20 0 1 0 94752538 4169728 81 18446744073709551615 4194304 4210620 140735996104016 140735996095456 140664960886018 0 0 0 0 0 0 0 17 1 0 0 0 0 0\n% grep 'Name\\|Pid\\|Sig' /proc/2031/status\nName:\temacsclient\nPid:\t2031\nPPid:\t2030\nTracerPid:\t0\nSigQ:\t0/28998\nSigPnd:\t0000000000000000\nSigBlk:\t0000000000000000\nSigIgn:\t0000000000000000\nSigCgt:\t0000000000000000\n%\n\nwhich I interpret to mean both processes were in process group\n2030, the cvs process ignored SIGINT and SIGQUIT, the emacsclient\nprocess neither ignored nor handled any signals, and neither\nprocess blocked any signals (not even SIGCHLD as system(3) would).\nWhen ^C in the terminal sent SIGINT to the process group, it\nterminated the emacsclient process only.\n\nIf git did the same thing as cvs here, i.e. ignore the signals in\nthe parent process only and check the exit status of the editor,\nI think that would be OK.\n"},{"id":"202803","messageId":"m2ehk0tt10.fsf@linux-m68k.org","threadId":"32049","inReplyTo":"87wqxs4o6f.fsf@Niukka.kon.iki.fi","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-11-11T08:58:51Z","receivedAt":"2012-11-11T08:58:51Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Kalle Olavi Niemitalo <kon@iki.fi> writes:\n\n> and neither process blocked any signals (not even SIGCHLD as system(3)\n> would).\n\nIf you don't have a SIGCHLD handler it won't matter anyway.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"202869","messageId":"20121111154846.GA13188@sigill.intra.peff.net","threadId":"32049","inReplyTo":"87wqxs4o6f.fsf@Niukka.kon.iki.fi","subject":"Re: What's cooking in git.git (Nov 2012, #02; Fri, 9)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T15:48:46Z","receivedAt":"2012-11-11T15:48:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 11, 2012 at 09:02:48AM +0200, Kalle Olavi Niemitalo wrote:\n\n> If git did the same thing as cvs here, i.e. ignore the signals in\n> the parent process only and check the exit status of the editor,\n> I think that would be OK.\n\nSilly me. When I thought through the impact of Paul's patch, I knew that\nwe would notice signal death of the editor. But I totally forgot to\nconsider that the blocked signal is inherited by the child process. I\nthink we just need to move the signal() call to after we've forked. Like\nthis (on top of Paul's patch):\n\ndiff --git a/editor.c b/editor.c\nindex 3ca361b..0ed23ce 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -38,11 +38,20 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \n \tif (strcmp(editor, \":\")) {\n \t\tconst char *args[] = { editor, path, NULL };\n+\t\tstruct child_process p;\n \t\tint ret;\n \n+\t\tmemset(&p, 0, sizeof(p));\n+\t\tp.argv = args;\n+\t\tp.env = env;\n+\t\tp.use_shell = 1;\n+\t\tif (start_command(&p) < 0)\n+\t\t\treturn error(\"unable to start editor '%s'\", editor);\n+\n \t\tsigchain_push(SIGINT, SIG_IGN);\n-\t\tret = run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env);\n+\t\tret = finish_command(&p);\n \t\tsigchain_pop(SIGINT);\n+\n \t\tif (ret)\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n\nNote that this will give you a slightly verbose message from git.\nPotentially we could notice editor death due to SIGINT and suppress the\nmessage, under the assumption that the user hit ^C and does not need to\nbe told.\n\n-Peff\n"},{"id":"202870","messageId":"20121111163100.GB13188@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111154846.GA13188@sigill.intra.peff.net","subject":"[PATCH 0/5] ignore SIGINT while editor runs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:31:00Z","receivedAt":"2012-11-11T16:31:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 11, 2012 at 10:48:46AM -0500, Jeff King wrote:\n\n> Silly me. When I thought through the impact of Paul's patch, I knew that\n> we would notice signal death of the editor. But I totally forgot to\n> consider that the blocked signal is inherited by the child process. I\n> think we just need to move the signal() call to after we've forked. Like\n> this (on top of Paul's patch):\n> [...]\n> Note that this will give you a slightly verbose message from git.\n> Potentially we could notice editor death due to SIGINT and suppress the\n> message, under the assumption that the user hit ^C and does not need to\n> be told.\n\nHere's a series that I think should resolve the situation for everybody.\n\n  [1/5]: launch_editor: refactor to use start/finish_command\n\nThe cleanup I sent out a few minutes ago.\n\n  [2/5]: launch_editor: ignore SIGINT while the editor has control\n\nPaul's patch rebased on my 1/5.\n\n  [3/5]: run-command: drop silent_exec_failure arg from wait_or_whine\n  [4/5]: run-command: do not warn about child death by SIGINT\n  [5/5]: launch_editor: propagate SIGINT from editor to git\n\nAct more like current git when the editor dies from SIGINT.\n\n-Peff\n"},{"id":"202878","messageId":"20121111165500.GA19850@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"[PATCH 1/5] launch_editor: refactor to use start/finish_command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:55:01Z","receivedAt":"2012-11-11T16:55:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The launch_editor function uses the convenient run_command_*\ninterface. Let's use the more flexible start_command and\nfinish_command functions, which will let us manipulate the\nparent state while we're waiting for the child to finish.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n editor.c | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/editor.c b/editor.c\nindex d834003..842f782 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -37,8 +37,16 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \n \tif (strcmp(editor, \":\")) {\n \t\tconst char *args[] = { editor, path, NULL };\n+\t\tstruct child_process p;\n \n-\t\tif (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env))\n+\t\tmemset(&p, 0, sizeof(p));\n+\t\tp.argv = args;\n+\t\tp.env = env;\n+\t\tp.use_shell = 1;\n+\t\tif (start_command(&p) < 0)\n+\t\t\treturn error(\"unable to start editor '%s'\", editor);\n+\n+\t\tif (finish_command(&p))\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n \t}\n-- \n1.8.0.207.gdf2154c\n"},{"id":"202889","messageId":"20121111165510.GB19850@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"[PATCH 2/5] launch_editor: ignore SIGINT while the editor has control","fromName":"Paul Fox","fromEmail":"pgf@foxharp.boston.ma.us","sentAt":"2012-11-11T16:55:11Z","receivedAt":"2012-11-11T16:55:11Z","isPatch":true,"sender":{"key":"pgf@foxharp.boston.ma.us","avatar":"https://avatars.githubusercontent.com/u/4249842?v=4"},"body":"The user's editor likely catches SIGINT (ctrl-C).  but if\nthe user spawns a command from the editor and uses ctrl-C to\nkill that command, the SIGINT will likely also kill git\nitself (depending on the editor, this can leave the terminal\nin an unusable state).\n\nSigned-off-by: Paul Fox <pgf@foxharp.boston.ma.us>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n editor.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/editor.c b/editor.c\nindex 842f782..28aae85 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"strbuf.h\"\n #include \"run-command.h\"\n+#include \"sigchain.h\"\n \n #ifndef DEFAULT_EDITOR\n #define DEFAULT_EDITOR \"vi\"\n@@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \tif (strcmp(editor, \":\")) {\n \t\tconst char *args[] = { editor, path, NULL };\n \t\tstruct child_process p;\n+\t\tint ret;\n \n \t\tmemset(&p, 0, sizeof(p));\n \t\tp.argv = args;\n@@ -46,7 +48,10 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tif (start_command(&p) < 0)\n \t\t\treturn error(\"unable to start editor '%s'\", editor);\n \n-\t\tif (finish_command(&p))\n+\t\tsigchain_push(SIGINT, SIG_IGN);\n+\t\tret = finish_command(&p);\n+\t\tsigchain_pop(SIGINT);\n+\t\tif (ret)\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n \t}\n-- \n1.8.0.207.gdf2154c\n"},{"id":"202879","messageId":"20121111165544.GC19850@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"[PATCH 3/5] run-command: drop silent_exec_failure arg from wait_or_whine","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:55:44Z","receivedAt":"2012-11-11T16:55:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We do not actually use this parameter; instead we complain\nfrom the child itself (for fork/exec) or from start_command\n(if we are using spawn on Windows).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nJust a cleanup I noticed while in the area.\n\n run-command.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 3b982e4..3aae270 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -226,7 +226,7 @@ static inline void set_cloexec(int fd)\n \t\tfcntl(fd, F_SETFD, flags | FD_CLOEXEC);\n }\n \n-static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)\n+static int wait_or_whine(pid_t pid, const char *argv0)\n {\n \tint status, code = -1;\n \tpid_t waiting;\n@@ -432,8 +432,7 @@ fail_pipe:\n \t\t * At this point we know that fork() succeeded, but execvp()\n \t\t * failed. Errors have been reported to our stderr.\n \t\t */\n-\t\twait_or_whine(cmd->pid, cmd->argv[0],\n-\t\t\t      cmd->silent_exec_failure);\n+\t\twait_or_whine(cmd->pid, cmd->argv[0]);\n \t\tfailed_errno = errno;\n \t\tcmd->pid = -1;\n \t}\n@@ -538,7 +537,7 @@ fail_pipe:\n \n int finish_command(struct child_process *cmd)\n {\n-\treturn wait_or_whine(cmd->pid, cmd->argv[0], cmd->silent_exec_failure);\n+\treturn wait_or_whine(cmd->pid, cmd->argv[0]);\n }\n \n int run_command(struct child_process *cmd)\n-- \n1.8.0.207.gdf2154c\n"},{"id":"202881","messageId":"20121111165654.GD19850@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"[PATCH 4/5] run-command: do not warn about child death by SIGINT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:56:54Z","receivedAt":"2012-11-11T16:56:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"SIGINT is not generally an interesting signal to the user,\nsince it is typically caused by them hitting \"^C\" or\notherwise telling their terminal to send the signal.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI thought about making this an optional parameter for run-command, but\nit seems like everybody would want this (and most callsites will\ncomplain about a failed command separately, anyway, so it is not like\nerrors would go unnoticed).\n\n run-command.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 3aae270..0527c61 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -242,7 +242,8 @@ static int wait_or_whine(pid_t pid, const char *argv0)\n \t\terror(\"waitpid is confused (%s)\", argv0);\n \t} else if (WIFSIGNALED(status)) {\n \t\tcode = WTERMSIG(status);\n-\t\terror(\"%s died of signal %d\", argv0, code);\n+\t\tif (code != SIGINT)\n+\t\t\terror(\"%s died of signal %d\", argv0, code);\n \t\t/*\n \t\t * This return value is chosen so that code & 0xff\n \t\t * mimics the exit code that a POSIX shell would report for\n-- \n1.8.0.207.gdf2154c\n"},{"id":"202882","messageId":"20121111165706.GE19850@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"[PATCH 5/5] launch_editor: propagate SIGINT from editor to git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:57:06Z","receivedAt":"2012-11-11T16:57:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We block SIGINT while the editor runs so that git is not\nkilled accidentally by a stray \"^C\" meant for the editor or\nits subprocesses. This works because most editors ignore\nSIGINT.\n\nHowever, some editor wrappers, like emacsclient, expect to\ndie due to ^C. We detect the signal death in the editor and\nproperly exit, but not before writing a useless error\nmessage to stderr. Instead, let's notice when the editor was\nkilled by SIGINT and just raise the signal on ourselves.\nThis skips the message and looks to our parent like we\nreceived SIGINT ourselves.\n\nThe end effect is that if the user's editor ignores SIGINT,\nwe will, too. And if it does not, then we will behave as if\nwe did not ignore it. That should make all users happy.\n\nNote that in the off chance that another part of git has\nignored SIGINT while calling launch_editor, we will still\nproperly detect and propagate the failed return code from\nthe editor (i.e., the worst case is that we generate the\nuseless error, not fail to notice the editor's death).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n editor.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/editor.c b/editor.c\nindex 28aae85..1275527 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tsigchain_push(SIGINT, SIG_IGN);\n \t\tret = finish_command(&p);\n \t\tsigchain_pop(SIGINT);\n+\t\tif (WIFSIGNALED(ret) && WTERMSIG(ret) == SIGINT)\n+\t\t\traise(SIGINT);\n \t\tif (ret)\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n-- \n1.8.0.207.gdf2154c\n"},{"id":"202883","messageId":"20121111165855.GA19962@sigill.intra.peff.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"[PATCH 2/5] launch_editor: ignore SIGINT while the editor has control","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-11T16:58:56Z","receivedAt":"2012-11-11T16:58:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Paul Fox <pgf@foxharp.boston.ma.us>\n\nThe user's editor likely catches SIGINT (ctrl-C).  but if\nthe user spawns a command from the editor and uses ctrl-C to\nkill that command, the SIGINT will likely also kill git\nitself (depending on the editor, this can leave the terminal\nin an unusable state).\n\nSigned-off-by: Paul Fox <pgf@foxharp.boston.ma.us>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nWhoops, my original sending of this actually had Paul in the email's\nFrom field, not in the pseudo-header of the commit. Apologies if you\nreceive an extra forged copy.\n\n editor.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/editor.c b/editor.c\nindex 842f782..28aae85 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"strbuf.h\"\n #include \"run-command.h\"\n+#include \"sigchain.h\"\n \n #ifndef DEFAULT_EDITOR\n #define DEFAULT_EDITOR \"vi\"\n@@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \tif (strcmp(editor, \":\")) {\n \t\tconst char *args[] = { editor, path, NULL };\n \t\tstruct child_process p;\n+\t\tint ret;\n \n \t\tmemset(&p, 0, sizeof(p));\n \t\tp.argv = args;\n@@ -46,7 +48,10 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tif (start_command(&p) < 0)\n \t\t\treturn error(\"unable to start editor '%s'\", editor);\n \n-\t\tif (finish_command(&p))\n+\t\tsigchain_push(SIGINT, SIG_IGN);\n+\t\tret = finish_command(&p);\n+\t\tsigchain_pop(SIGINT);\n+\t\tif (ret)\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n \t}\n-- \n1.8.0.207.gdf2154c\n"},{"id":"202908","messageId":"CAMP44s3_CDHhcEe_ponW__=TdG_c1DQyLU1VG4UCzOC=MxW9mQ@mail.gmail.com","threadId":"32049","inReplyTo":"20121111165544.GC19850@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] run-command: drop silent_exec_failure arg from wait_or_whine","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-11T18:13:00Z","receivedAt":"2012-11-11T18:13:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Nov 11, 2012 at 5:55 PM, Jeff King <peff@peff.net> wrote:\n> We do not actually use this parameter; instead we complain\n> from the child itself (for fork/exec) or from start_command\n> (if we are using spawn on Windows).\n\nFWIW I noticed the same while looking at that code. Looks good to me.\n\n-- \nFelipe Contreras\n"},{"id":"202911","messageId":"20121111182703.167E92E8B6A@grass.foxharp.boston.ma.us","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] ignore SIGINT while editor runs","fromName":"Paul Fox","fromEmail":"pgf@foxharp.boston.ma.us","sentAt":"2012-11-11T18:27:03Z","receivedAt":"2012-11-11T18:27:03Z","isPatch":true,"sender":{"key":"pgf@foxharp.boston.ma.us","avatar":"https://avatars.githubusercontent.com/u/4249842?v=4"},"body":"jeff wrote:\n > On Sun, Nov 11, 2012 at 10:48:46AM -0500, Jeff King wrote:\n > \n > > Silly me. When I thought through the impact of Paul's patch, I knew that\n > > we would notice signal death of the editor. But I totally forgot to\n > > consider that the blocked signal is inherited by the child process. I\n > > think we just need to move the signal() call to after we've forked. Like\n > > this (on top of Paul's patch):\n > > [...]\n > > Note that this will give you a slightly verbose message from git.\n > > Potentially we could notice editor death due to SIGINT and suppress the\n > > message, under the assumption that the user hit ^C and does not need to\n > > be told.\n > \n > Here's a series that I think should resolve the situation for everybody.\n\nthanks!  i've tested -- this certainly scratches my initial itch.\n\nack,\npaul\n\n > \n >   [1/5]: launch_editor: refactor to use start/finish_command\n > \n > The cleanup I sent out a few minutes ago.\n > \n >   [2/5]: launch_editor: ignore SIGINT while the editor has control\n > \n > Paul's patch rebased on my 1/5.\n > \n >   [3/5]: run-command: drop silent_exec_failure arg from wait_or_whine\n >   [4/5]: run-command: do not warn about child death by SIGINT\n >   [5/5]: launch_editor: propagate SIGINT from editor to git\n > \n > Act more like current git when the editor dies from SIGINT.\n > \n > -Peff\n > --\n > To unsubscribe from this list: send the line \"unsubscribe git\" in\n > the body of a message to majordomo@vger.kernel.org\n > More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\n=---------------------\n paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 56.3 degrees)\n"},{"id":"202914","messageId":"20121111191520.GA9474@shrek.podlesie.net","threadId":"32049","inReplyTo":"20121111163100.GB13188@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] ignore SIGINT while editor runs","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-11T19:15:20Z","receivedAt":"2012-11-11T19:15:20Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Sun, Nov 11, 2012 at 11:31:00AM -0500, Jeff King wrote:\n> \n> Here's a series that I think should resolve the situation for everybody.\n> \n>   [1/5]: launch_editor: refactor to use start/finish_command\n> \n> The cleanup I sent out a few minutes ago.\n> \n>   [2/5]: launch_editor: ignore SIGINT while the editor has control\n> \n> Paul's patch rebased on my 1/5.\n> \n>   [3/5]: run-command: drop silent_exec_failure arg from wait_or_whine\n>   [4/5]: run-command: do not warn about child death by SIGINT\n>   [5/5]: launch_editor: propagate SIGINT from editor to git\n> \n> Act more like current git when the editor dies from SIGINT.\n> \n\nLooks ok, but what about SIGQUIT? Some editors like GNU ed (0.4 and 1.6)\nignore SIGQUIT, and after SIGQUIT git dies, but editor is still running.\nAfter pressing any key ed receives -EIO and prints \"stdin: Input/output\nerror\". GNU ed 1.6 then exits, but ed 0.4 prints this error forever.\nMaybe git should kill the editor in such case?\n\nKrzysiek\n"},{"id":"202915","messageId":"50A00116.8060604@kdbg.org","threadId":"32049","inReplyTo":"20121111165706.GE19850@sigill.intra.peff.net","subject":"Re: [PATCH 5/5] launch_editor: propagate SIGINT from editor to git","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-11-11T19:48:38Z","receivedAt":"2012-11-11T19:48:38Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.11.2012 17:57, schrieb Jeff King:\n> @@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n>  \t\tsigchain_push(SIGINT, SIG_IGN);\n>  \t\tret = finish_command(&p);\n>  \t\tsigchain_pop(SIGINT);\n> +\t\tif (WIFSIGNALED(ret) && WTERMSIG(ret) == SIGINT)\n> +\t\t\traise(SIGINT);\n\nThe return value of finish_command() is already a digested version of\nwaitpid's status value. According to\nDocumentation/technical/api-run-command.txt:\n\n. If the program terminated due to a signal, then the return value is\nthe signal number - 128, ...\n\nthe correct condition would be\n\n\t\tif (ret == SIGINT - 128)\n\n-- Hannes\n"},{"id":"202917","messageId":"20121111202419.7602E2E8B6A@grass.foxharp.boston.ma.us","threadId":"32049","inReplyTo":"20121111191520.GA9474@shrek.podlesie.net","subject":"Re: [PATCH 0/5] ignore SIGINT while editor runs","fromName":"Paul Fox","fromEmail":"pgf@foxharp.boston.ma.us","sentAt":"2012-11-11T20:24:19Z","receivedAt":"2012-11-11T20:24:19Z","isPatch":true,"sender":{"key":"pgf@foxharp.boston.ma.us","avatar":"https://avatars.githubusercontent.com/u/4249842?v=4"},"body":"krzysztof wrote:\n > On Sun, Nov 11, 2012 at 11:31:00AM -0500, Jeff King wrote:\n > > \n > > Here's a series that I think should resolve the situation for everybody.\n > > \n > >   [1/5]: launch_editor: refactor to use start/finish_command\n > > \n > > The cleanup I sent out a few minutes ago.\n > > \n > >   [2/5]: launch_editor: ignore SIGINT while the editor has control\n > > \n > > Paul's patch rebased on my 1/5.\n > > \n > >   [3/5]: run-command: drop silent_exec_failure arg from wait_or_whine\n > >   [4/5]: run-command: do not warn about child death by SIGINT\n > >   [5/5]: launch_editor: propagate SIGINT from editor to git\n > > \n > > Act more like current git when the editor dies from SIGINT.\n > > \n > \n > Looks ok, but what about SIGQUIT? Some editors like GNU ed (0.4 and 1.6)\n > ignore SIGQUIT, and after SIGQUIT git dies, but editor is still running.\n > After pressing any key ed receives -EIO and prints \"stdin: Input/output\n > error\". GNU ed 1.6 then exits, but ed 0.4 prints this error forever.\n > Maybe git should kill the editor in such case?\n\nthere's certainly lots of precedent for treating SIGINT and SIGQUIT\nthe same.  but there's also some merit to saying that if the user\nknows to send SIGQUIT instead of SIGINT, they may well have a reason. \n(after all, if we always treat them the same, there's no point in\nhaving both.)\n\nthe em editor (linus' microemacs) behaves as you describe ed 0.4 does,\nexcept without the error message -- it just spins silently getting EIO\nfrom reading stdin.  i think em needs to be fixed, and it sounds like\nGNU ed already has been.  (unless i misunderstand the relationship of\n0.4 and 1.6.)\n\npaul\n\n > \n > Krzysiek\n > --\n > To unsubscribe from this list: send the line \"unsubscribe git\" in\n > the body of a message to majordomo@vger.kernel.org\n > More majordomo info at  http://vger.kernel.org/majordomo-info.html\n\n=---------------------\n paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 57.2 degrees)\n"},{"id":"202918","messageId":"20121111204305.GA20599@shrek.podlesie.net","threadId":"32049","inReplyTo":"20121111202419.7602E2E8B6A@grass.foxharp.boston.ma.us","subject":"Re: [PATCH 0/5] ignore SIGINT while editor runs","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-11T20:43:05Z","receivedAt":"2012-11-11T20:43:05Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Sun, Nov 11, 2012 at 03:24:19PM -0500, Paul Fox wrote:\n> krzysztof wrote:\n>  > Looks ok, but what about SIGQUIT? Some editors like GNU ed (0.4 and 1.6)\n>  > ignore SIGQUIT, and after SIGQUIT git dies, but editor is still running.\n>  > After pressing any key ed receives -EIO and prints \"stdin: Input/output\n>  > error\". GNU ed 1.6 then exits, but ed 0.4 prints this error forever.\n>  > Maybe git should kill the editor in such case?\n> \n> there's certainly lots of precedent for treating SIGINT and SIGQUIT\n> the same.  but there's also some merit to saying that if the user\n> knows to send SIGQUIT instead of SIGINT, they may well have a reason. \n> (after all, if we always treat them the same, there's no point in\n> having both.)\n\nThat's why I'm proposing in case of SIGQUIT just killing the editor\n(SIGTERM is sufficient for ed).\n\nSo git will ignore SIGINT, but die on SIGQUIT (and kill editor\nthat ignores SIGQUIT).\n\n> \n> the em editor (linus' microemacs) behaves as you describe ed 0.4 does,\n> except without the error message -- it just spins silently getting EIO\n> from reading stdin.  i think em needs to be fixed, and it sounds like\n> GNU ed already has been.  (unless i misunderstand the relationship of\n> 0.4 and 1.6.)\n\nYes, the version 1.6 is fixed, it just prints an error once and exits.\n\nKrzysiek\n"},{"id":"202925","messageId":"m2mwynkd1w.fsf@igel.home","threadId":"32049","inReplyTo":"20121111204305.GA20599@shrek.podlesie.net","subject":"Re: [PATCH 0/5] ignore SIGINT while editor runs","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-11-11T22:08:43Z","receivedAt":"2012-11-11T22:08:43Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> That's why I'm proposing in case of SIGQUIT just killing the editor\n> (SIGTERM is sufficient for ed).\n>\n> So git will ignore SIGINT, but die on SIGQUIT (and kill editor\n> that ignores SIGQUIT).\n\nsystem(3) also ignores SIGQUIT.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"202974","messageId":"7v4nkuk966.fsf@alter.siamese.dyndns.org","threadId":"32049","inReplyTo":"20121111165510.GB19850@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-12T17:44:49Z","receivedAt":"2012-11-12T17:44:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"How did this message happen?\n\n    Subject: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control\n    To: Kalle Olavi Niemitalo <kon@iki.fi>\n    Cc: Paul Fox <pgf@foxharp.boston.ma.us>, git@vger.kernel.org\n    Date: Sun, 11 Nov 2012 11:55:11 -0500\n    Message-ID: <20121111165510.GB19850@sigill.intra.peff.net>\n    References: <20121111163100.GB13188@sigill.intra.peff.net>\n\n    The user's editor likely catches SIGINT (ctrl-C).  but if\n    the user spawns a command from the editor and uses ctrl-C to\n    kill that command, the SIGINT will likely also kill git\n    itself (depending on the editor, this can leave the terminal\n    in an unusable state).\n\n    Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>\n    Signed-off-by: Jeff King <peff@peff.net>\n    ---\n\nJudging from S-o-b, message-id and EHLO, I think this was sent by\nPeff, but came without Sender: or anything.\n\nJust being curious.\n"},{"id":"202987","messageId":"20121112194738.GC4623@sigill.intra.peff.net","threadId":"32049","inReplyTo":"7v4nkuk966.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-12T19:47:38Z","receivedAt":"2012-11-12T19:47:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2012 at 09:44:49AM -0800, Junio C Hamano wrote:\n\n> How did this message happen?\n> \n>     Subject: [PATCH 2/5] launch_editor: ignore SIGINT while the editor has control\n>     To: Kalle Olavi Niemitalo <kon@iki.fi>\n>     Cc: Paul Fox <pgf@foxharp.boston.ma.us>, git@vger.kernel.org\n>     Date: Sun, 11 Nov 2012 11:55:11 -0500\n>     Message-ID: <20121111165510.GB19850@sigill.intra.peff.net>\n>     References: <20121111163100.GB13188@sigill.intra.peff.net>\n> \n>     The user's editor likely catches SIGINT (ctrl-C).  but if\n>     the user spawns a command from the editor and uses ctrl-C to\n>     kill that command, the SIGINT will likely also kill git\n>     itself (depending on the editor, this can leave the terminal\n>     in an unusable state).\n> \n>     Signed-off-by: Paul Fox <pgf@foxharp.boston.ma.us>\n>     Signed-off-by: Jeff King <peff@peff.net>\n>     ---\n> \n> Judging from S-o-b, message-id and EHLO, I think this was sent by\n> Peff, but came without Sender: or anything.\n> \n> Just being curious.\n\nI screwed up when sending out the series and did not properly move\nPaul's \"From\" address from the email header down to the body. It is not\na git or send-email screw-up; I load format-patch output directly into\nmutt as a template. Since I do not often send out other people's\npatches, I never bothered to write a script to migrate the \"from\" into\nthe body automatically.\n\n-Peff\n"},{"id":"204358","messageId":"20121130202435.GA7933@sigill.intra.peff.net","threadId":"32049","inReplyTo":"50A00116.8060604@kdbg.org","subject":"Re: [PATCH 5/5] launch_editor: propagate SIGINT from editor to git","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-30T20:24:35Z","receivedAt":"2012-11-30T20:24:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 11, 2012 at 08:48:38PM +0100, Johannes Sixt wrote:\n\n> Am 11.11.2012 17:57, schrieb Jeff King:\n> > @@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n> >  \t\tsigchain_push(SIGINT, SIG_IGN);\n> >  \t\tret = finish_command(&p);\n> >  \t\tsigchain_pop(SIGINT);\n> > +\t\tif (WIFSIGNALED(ret) && WTERMSIG(ret) == SIGINT)\n> > +\t\t\traise(SIGINT);\n> \n> The return value of finish_command() is already a digested version of\n> waitpid's status value. According to\n> Documentation/technical/api-run-command.txt:\n> \n> . If the program terminated due to a signal, then the return value is\n> the signal number - 128, ...\n> \n> the correct condition would be\n> \n> \t\tif (ret == SIGINT - 128)\n\nYeah, that is the same thing as WTERMSIG (which uses \"ret & 0x7f\") for\nthe range of -127..-1. I do not mind changing it to match run-command's\nstated output, but I am curious whether there are systems where WTERMSIG\nis not defined in the same way, and the code would break.\n\n-Peff\n"}]}