{"thread":{"id":"29092","subject":"What's cooking in git.git (Dec 2011, #02; Mon, 5)","startedAt":"2011-12-06T05:01:52Z","lastAt":"2011-12-08T21:25:56Z","messageCount":45,"participants":["Junio C Hamano","Ramkumar Ramachandra","Jeff King","Nguyen Thai Ngoc Duy","Erik Faye-Lund","Luke Diamand","Jonathan Nieder","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"180338","messageId":"7v8vmqi98f.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":null,"subject":"What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T05:01:52Z","receivedAt":"2011-12-06T05:01:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are the topics that have been cooking.  Commits prefixed with '-' are\nonly in 'pu' (proposed updates) while commits prefixed with '+' are in\n'next'.\n\nThe tip of 'next' will be rewound soonish, kicking a handful of topics\nback to 'pu' that are important enough to get them right rather than\npiling \"oops, that was a mistake\" fix-ups in 'next'.\n\nAll of the topics in Stalled section will be purged without prejudice;\nowners of respective topics are free to re-submit after addressing the\nissues raised during the previous reviews and re-ignite the discussion.\n\nHere are the repositories that have my integration branches:\n\nWith maint, master, next, pu, todo:\n\n        git://git.kernel.org/pub/scm/git/git.git\n        git://repo.or.cz/alt-git.git\n        https://code.google.com/p/git-core/\n        https://github.com/git/git\n\nWith only maint and master:\n\n        git://git.sourceforge.jp/gitroot/git-core/git.git\n        git://git-core.git.sourceforge.net/gitroot/git-core/git-core\n\nWith all the topics and integration branches:\n\n        https://github.com/gitster/git\n\nThe preformatted documentation in HTML and man format are found in:\n\n        git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/\n        git://repo.or.cz/git-{htmldocs,manpages}.git/\n        https://code.google.com/p/git-{htmldocs,manpages}.git/\n        https://github.com/gitster/git-{htmldocs,manpages}.git/\n\n--------------------------------------------------\n[New Topics]\n\n* bc/maint-apply-check-no-patch (2011-12-05) 2 commits\n - builtin/apply.c: report error on failure to recognize input\n - t/t4131-apply-fake-ancestor.sh: fix broken test\n\nWill merge to 'next'.\n\n--------------------------------------------------\n[Graduated to \"master\"]\n\n* ab/clang-lints (2011-11-06) 2 commits\n  (merged to 'next' on 2011-11-13 at a573aec)\n + cast variable in call to free() in builtin/diff.c and submodule.c\n + apply: get rid of useless x < 0 comparison on a size_t type\n\n* gh/userdiff-matlab (2011-11-15) 1 commit\n  (merged to 'next' on 2011-11-18 at 10cd275)\n + Add built-in diff patterns for MATLAB code\n\n* jc/maint-pack-object-cycle (2011-11-16) 1 commit\n  (merged to 'next' on 2011-11-18 at 3715a81)\n + pack-object: tolerate broken packs that have duplicated objects\n\nMake the client side more robust against bogus pack stream; the problem\nwas discovered by accident while repacking a clone obtained from somewhat\nbuggy test server.\n\n* jc/index-pack-reject-dups (2011-11-16) 1 commit\n  (merged to 'next' on 2011-11-18 at 2090221)\n + receive-pack, fetch-pack: reject bogus pack that records objects twice\n\nAnd this is the prevention to reject such pack stream in the first place.\n\n* jk/refresh-porcelain-output (2011-11-18) 3 commits\n  (merged to 'next' on 2011-11-18 at 872f25e)\n + refresh_index: make porcelain output more specific\n + refresh_index: rename format variables\n + read-cache: let refresh_cache_ent pass up changed flags\n\n* mf/curl-select-fdset (2011-11-04) 4 commits\n  (merged to 'next' on 2011-11-06 at a49516f)\n + http: drop \"local\" member from request struct\n + http.c: Rely on select instead of tracking whether data was received\n + http.c: Use timeout suggested by curl instead of fixed 50ms timeout\n + http.c: Use curl_multi_fdset to select on curl fds instead of just sleeping\n\nReduces unnecessary waits.\n\n* na/strtoimax (2011-11-05) 3 commits\n  (merged to 'next' on 2011-11-14 at b64e1cb)\n + Support sizes >=2G in various config options accepting 'g' sizes.\n + Compatibility: declare strtoimax() under NO_STRTOUMAX\n + Add strtoimax() compatibility function.\n\n* nd/fsck-progress (2011-11-06) 4 commits\n  (merged to 'next' on 2011-11-13 at 8831811)\n + fsck: print progress\n + fsck: avoid reading every object twice\n + verify_packfile(): check as many object as possible in a pack\n + fsck: return error code when verify_pack() goes wrong\n\n* nd/maint-ignore-exclude (2011-11-28) 1 commit\n + checkout,merge: loosen overwriting untracked file check based on info/exclude\n (this branch is used by nd/ignore-might-be-precious.)\n\n* nd/misc-cleanups (2011-10-27) 6 commits\n  (merged to 'next' on 2011-10-28 at 2527a49)\n + unpack_object_header_buffer(): clear the size field upon error\n + tree_entry_interesting: make use of local pointer \"item\"\n + tree_entry_interesting(): give meaningful names to return values\n + read_directory_recursive: reduce one indentation level\n + get_tree_entry(): do not call find_tree_entry() on an empty tree\n + tree-walk.c: do not leak internal structure in tree_entry_len()\n\n* nd/prune-progress (2011-11-07) 3 commits\n  (merged to 'next' on 2011-11-13 at c5722ac)\n + reachable: per-object progress\n + prune: handle --progress/no-progress\n + prune: show progress while marking reachable objects\n\n* vr/git-merge-doc (2011-11-21) 1 commit\n + Show error for 'git merge' with unset merge.defaultToUpstream\n\n* vr/msvc (2011-10-31) 3 commits\n  (merged to 'next' on 2011-11-14 at f46d021)\n + MSVC: Remove unneeded header stubs\n + Compile fix for MSVC: Include <io.h>\n + Compile fix for MSVC: Do not include sys/resources.h\n\n--------------------------------------------------\n[Stalled]\n\nAll of the topics in Stalled section will be discarded soonish (see the\ncover).\n\n* hv/submodule-merge-search (2011-10-13) 4 commits\n - submodule.c: make two functions static\n - allow multiple calls to submodule merge search for the same path\n - push: Don't push a repository with unpushed submodules\n - push: teach --recurse-submodules the on-demand option\n\nWhat the topic aims to achieve may make sense, but the implementation\nlooked somewhat suboptimal.\n\n* sr/transport-helper-fix-rfc (2011-07-19) 2 commits\n - t5800: point out that deleting branches does not work\n - t5800: document inability to push new branch with old content\n\nSee comments on sr/fix-fast-export-tips topic.\n\n* sr/fix-fast-export-tips (2011-11-05) 3 commits\n - fast-export: output reset command for commandline revs\n - fast-export: do not refer to non-existing marks\n - t9350: point out that refs are not updated correctly\n\nThe bottom commit from the stalled sr/transport-helper-fix-rfc topic is\nfixed with this. It may make sense to drop the other topic and include\nthat commit in this series.\n\nThe command line parser is still too lax and accepts malformed input, but\nthis is a step in the right direction and tightening the command line now\nshould be doable without a low level surgery that touches codepaths that\nare unrelated to the command line processing like the previous attempt\nused to do.\n\n* jc/lookup-object-hash (2011-08-11) 6 commits\n - object hash: replace linear probing with 4-way cuckoo hashing\n - object hash: we know the table size is a power of two\n - object hash: next_size() helper for readability\n - pack-objects --count-only\n - object.c: remove duplicated code for object hashing\n - object.c: code movement for readability\n\nI do not think there is anything fundamentally wrong with this series, but\nthe risk of breakage far outweighs observed performance gain in one\nparticular workload.\n\n* jc/verbose-checkout (2011-10-16) 2 commits\n - checkout -v: give full status output after switching branches\n - checkout: move the local changes report to the end\n\nThis is just to leave a record that the reason why we do not do this not\nbecause we are incapable of coding this, but because it is not a good idea\nto do this. I suspect people who are new to git that might think they need\nit would soon realize the don't.\n\nWill drop.\n\n* eh/grep-scale-to-cpunum (2011-11-05) 1 commit\n - grep: detect number of CPUs for thread spawning\n\nKills I/O parallelism and needs to be improved or discarded.\n\n* jc/commit-tree-extra (2011-11-12) 2 commits\n - commit-tree: teach -C <extra-commit>\n - commit-tree: teach -x <extra>\n (this branch uses jc/pull-signed-tag; is tangled with jc/signed-commit.)\n\nNot absolutely needed; parked in 'pu' but may drop.\n\n* cb/daemon-permission-errors (2011-10-17) 2 commits\n - daemon: report permission denied error to clients\n - daemon: add tests\n\nThe tip commit might be loosening things a bit too much.\n\n--------------------------------------------------\n[Cooking]\n\n* sg/complete-refs (2011-10-21) 9 commits\n  (merged to 'next' on 2011-10-26 at d65e2b4)\n + completion: remove broken dead code from __git_heads() and __git_tags()\n + completion: fast initial completion for config 'remote.*.fetch' value\n + completion: improve ls-remote output filtering in __git_refs_remotes()\n + completion: query only refs/heads/ in __git_refs_remotes()\n + completion: support full refs from remote repositories\n + completion: improve ls-remote output filtering in __git_refs()\n + completion: make refs completion consistent for local and remote repos\n + completion: optimize refs completion\n + completion: document __gitcomp()\n\nWill merge to 'master.\n\n* aw/rebase-i-stop-on-failure-to-amend (2011-11-30) 1 commit\n - rebase -i: interrupt rebase when \"commit --amend\" failed during \"reword\"\n\nWill merge to 'next'.\n\n* jc/split-blob (2011-12-01) 6 commits\n . WIP (streaming chunked)\n - chunked-object: fallback checkout codepaths\n - bulk-checkin: support chunked-object encoding\n - bulk-checkin: allow the same data to be multiply hashed\n - new representation types in the packstream\n - varint-in-pack: refactor varint encoding/decoding\n (this branch uses jc/stream-to-pack.)\n\nNot ready.\n\n* jh/fast-import-notes (2011-11-28) 3 commits\n - fast-import: Fix incorrect fanout level when modifying existing notes refs\n - t9301: Add 2nd testcase exposing bugs in fast-import's notes fanout handling\n - t9301: Fix testcase covering up a bug in fast-import's notes fanout handling\n\nWill merge to 'next'.\n\n* ld/p4-labels-branches (2011-11-30) 4 commits\n - git-p4: importing labels should cope with missing owner\n - git-p4: add test for p4 labels\n - git-p4: cope with labels with empty descriptions\n - git-p4: handle p4 branches and labels containing shell chars\n\nI understand this has been retracted---please correct me otherwise.\nWill discard, expecting a reroll.\n\n* tj/maint-imap-send-remove-unused (2011-11-23) 2 commits\n - Merge branch 'maint' into tj/imap-send-remove-unused\n - imap-send: Remove unused 'use_namespace' variable\n\nWill merge to 'next'.\n\n* cn/maint-lf-to-crlf-filter (2011-11-28) 1 commit\n - convert: track state in LF-to-CRLF filter\n\nWill merge to 'next' after taking another look.\n\n* jn/branch-move-to-self (2011-11-28) 2 commits\n - Allow checkout -B <current-branch> to update the current branch\n - branch: allow a no-op \"branch -M <current-branch> HEAD\"\n\nWill merge to 'next' after taking another look.\n\n* jk/credentials (2011-11-28) 20 commits\n - fixup! 034c066e\n - compat/getpass: add a /dev/tty implementation\n - credential: use git_prompt instead of git_getpass\n - prompt: add PROMPT_ECHO flag\n - stub out getpass_echo function\n - refactor git_getpass into generic prompt function\n - move git_getpass to its own source file\n - t: add test harness for external credential helpers\n - credentials: add \"store\" helper\n - strbuf: add strbuf_add*_urlencode\n - credentials: add \"cache\" helper\n - docs: end-user documentation for the credential subsystem\n - credential: make relevance of http path configurable\n - credential: add credential.*.username\n - credential: apply helper config\n - http: use credential API to get passwords\n - credential: add function for parsing url components\n - introduce credentials API\n - t5550: fix typo\n - test-lib: add test_config_global variant\n\nExpecting a reroll?\n\n* nd/ignore-might-be-precious (2011-11-28) 2 commits\n - checkout,merge: disallow overwriting ignored files with --no-overwrite-ignore\n - Merge branch 'nd/maint-ignore-exclude' into nd/ignore-might-be-precious\n\nWill merge to 'next' after taking another look.\n\n* jk/upload-archive-use-start-command (2011-11-21) 1 commit\n - upload-archive: use start_command instead of fork\n\nWhat's the status of this one?\n\n* jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit\n - archive: don't let remote clients get unreachable commits\n (this branch is used by jk/maint-upload-archive.)\n\n* jk/maint-upload-archive (2011-11-21) 1 commit\n - Merge branch 'jk/maint-1.6.2-upload-archive' into jk/maint-upload-archive\n (this branch uses jk/maint-1.6.2-upload-archive.)\n\nWill merge to 'next' after taking another look.\n\n* ab/enable-i18n (2011-12-05) 2 commits\n - Makefile: Add missing \"sparse object\" to gettext target-specific rule\n - i18n: add infrastructure for translating Git with gettext\n\nWill merge to 'next' after squashing the two and re-sending to the list\nfor review comments.\n\n* nd/resolve-ref (2011-12-05) 2 commits\n  (merged to 'next' on 2011-12-05 at cc79e86)\n + Copy resolve_ref() return value for longer use\n + Convert many resolve_ref() calls to read_ref*() and ref_exists()\n\nWill merge to 'master'.\n\n* jc/signed-commit (2011-11-29) 5 commits\n - gpg-interface: allow use of a custom GPG binary\n - pretty: %G[?GS] placeholders\n - test \"commit -S\" and \"log --show-signature\"\n - log: --show-signature\n - commit: teach --gpg-sign option\n (this branch uses jc/pull-signed-tag; is tangled with jc/commit-tree-extra.)\n\nNot exactly urgent.\nWill concentrate on getting jc/pull-signed-tag in first.\n\n* jc/pull-signed-tag (2011-11-12) 15 commits\n  (merged to 'next' on 2011-11-14 at 25e8838)\n + commit-tree: teach -m/-F options to read logs from elsewhere\n + commit-tree: update the command line parsing\n + commit: teach --amend to carry forward extra headers\n + merge: force edit and no-ff mode when merging a tag object\n + commit: copy merged signed tags to headers of merge commit\n + merge: record tag objects without peeling in MERGE_HEAD\n + merge: make usage of commit->util more extensible\n + fmt-merge-msg: Add contents of merged tag in the merge message\n + fmt-merge-msg: package options into a structure\n + fmt-merge-msg: avoid early returns\n + refs DWIMmery: use the same rule for both \"git fetch\" and others\n + fetch: allow \"git fetch $there v1.0\" to fetch a tag\n + merge: notice local merging of tags and keep it unwrapped\n + fetch: do not store peeled tag object names in FETCH_HEAD\n + Split GPG interface into its own helper library\n (this branch is used by jc/commit-tree-extra and jc/signed-commit.)\n\nAllow pulling/merging a signed tag instead of a branch tip, and record\nthe GPG signature in the merge commit for later audit.\n\nWill merge to 'master'.\n\n* jc/request-pull-show-head-4 (2011-11-09) 12 commits\n  (merged to 'next' on 2011-11-13 at e473fd2)\n + request-pull: use the annotated tag contents\n  (merged to 'next' on 2011-10-15 at 7e340ff)\n + fmt-merge-msg.c: Fix an \"dubious one-bit signed bitfield\" sparse error\n  (merged to 'next' on 2011-10-10 at 092175e)\n + environment.c: Fix an sparse \"symbol not declared\" warning\n + builtin/log.c: Fix an \"Using plain integer as NULL pointer\" warning\n  (merged to 'next' on 2011-10-07 at fcaeca0)\n + fmt-merge-msg: use branch.$name.description\n  (merged to 'next' on 2011-10-06 at fa5e0fe)\n + request-pull: use the branch description\n + request-pull: state what commit to expect\n + request-pull: modernize style\n + branch: teach --edit-description option\n + format-patch: use branch description in cover letter\n + branch: add read_branch_desc() helper function\n + Merge branch 'bk/ancestry-path' into jc/branch-desc\n\nAllow setting \"description\" for branches and use it to help communications\nbetween humans in various workflow elements. It also allows requesting for\na signed tag to be pulled and shows the tag message in the generated message.\n\nWill merge to 'master'.\n\n* ab/pull-rebase-config (2011-11-07) 1 commit\n  (merged to 'next' on 2011-11-13 at 72bb2d5)\n + pull: introduce a pull.rebase option to enable --rebase\n\nWill merge to 'master'.\n\n* jc/stream-to-pack (2011-12-01) 5 commits\n - bulk-checkin: replace fast-import based implementation\n - csum-file: introduce sha1file_checkpoint\n - finish_tmp_packfile(): a helper function\n - create_tmp_packfile(): a helper function\n - write_pack_header(): a helper function\n (this branch is used by jc/split-blob.)\n\nTeaches \"git add\" to send large-ish blob data straight to a packfile.\nThis is a continuation to the \"large file support\" topic. The codepath to\nmove data from worktree to repository is made aware of streaming, just\nlike the checkout codepath that goes the other way, which was done in the\nprevious \"large file support\" topic in the 1.7.7 cycle.\n\nWill merge to 'next'.\n\n* jn/gitweb-side-by-side-diff (2011-10-31) 8 commits\n - gitweb: Add navigation to select side-by-side diff\n - gitweb: Use href(-replay=>1,...) for formats links in \"commitdiff\"\n - t9500: Add basic sanity tests for side-by-side diff in gitweb\n - t9500: Add test for handling incomplete lines in diff by gitweb\n - gitweb: Give side-by-side diff extra CSS styling\n - gitweb: Add a feature to show side-by-side diff\n - gitweb: Extract formatting of diff chunk header\n - gitweb: Refactor diff body line classification\n\nReplaces a series from Kato Kazuyoshi on the same topic.\nIs this ready for 'next'?\n\n* rs/allocate-cache-entry-individually (2011-10-26) 2 commits\n  (merged to 'next' on 2011-10-27 at 2e4acd6)\n + cache.h: put single NUL at end of struct cache_entry\n + read-cache.c: allocate index entries individually\n\nWill merge to 'master'.\n\n* rr/revert-cherry-pick (2011-10-23) 5 commits\n  (merged to 'next' on 2011-10-26 at 27b7496)\n + revert: simplify communicating command-line arguments\n + revert: allow mixed pick and revert instructions\n + revert: make commit subjects in insn sheet optional\n + revert: simplify getting commit subject in format_todo()\n + revert: free msg in format_todo()\n\nThe internals of \"git revert/cherry-pick\" has been further refactored to\nserve as the basis for the sequencer.\n\nIs this ready for 'master', or should we eject it to 'pu', expecing a\nre-roll?\n\n* mh/ref-api-2 (2011-11-16) 15 commits\n  (merged to 'next' on 2011-11-16 at 511457f)\n + refs: loosen over-strict \"format\" check\n  (merged to 'next' on 2011-10-19 at cc89f0e)\n + resolve_gitlink_ref_recursive(): change to work with struct ref_cache\n + Pass a (ref_cache *) to the resolve_gitlink_*() helper functions\n + resolve_gitlink_ref(): improve docstring\n + get_ref_dir(): change signature\n + refs: change signatures of get_packed_refs() and get_loose_refs()\n + is_dup_ref(): extract function from sort_ref_array()\n + add_ref(): add docstring\n + parse_ref_line(): add docstring\n + is_refname_available(): remove the \"quiet\" argument\n + clear_ref_array(): rename from free_ref_array()\n + refs: rename parameters result -> sha1\n + refs: rename \"refname\" variables\n + struct ref_entry: document name member\n + cache.h: add comments for git_path() and git_path_submodule()\n (this branch is tangled with mh/ref-api-3.)\n\nExpecting a re-roll; will eject from 'pu' so that the topic can lose the\nhot-fix at the tip.\n\n* mh/ref-api-3 (2011-11-16) 26 commits\n  (merged to 'next' on 2011-11-16 at cc76151)\n + refs: loosen over-strict \"format\" check\n  (merged to 'next' on 2011-10-23 at 92e2d35)\n + is_refname_available(): reimplement using do_for_each_ref_in_array()\n + names_conflict(): simplify implementation\n + names_conflict(): new function, extracted from is_refname_available()\n + repack_without_ref(): reimplement using do_for_each_ref_in_array()\n + do_for_each_ref_in_array(): new function\n + do_for_each_ref(): correctly terminate while processesing extra_refs\n + add_ref(): take a (struct ref_entry *) parameter\n + create_ref_entry(): extract function from add_ref()\n + parse_ref_line(): add a check that the refname is properly formatted\n + repack_without_ref(): remove temporary\n + Rename another local variable name -> refname\n  (merged to 'next' on 2011-10-19 at cc89f0e)\n + resolve_gitlink_ref_recursive(): change to work with struct ref_cache\n + Pass a (ref_cache *) to the resolve_gitlink_*() helper functions\n + resolve_gitlink_ref(): improve docstring\n + get_ref_dir(): change signature\n + refs: change signatures of get_packed_refs() and get_loose_refs()\n + is_dup_ref(): extract function from sort_ref_array()\n + add_ref(): add docstring\n + parse_ref_line(): add docstring\n + is_refname_available(): remove the \"quiet\" argument\n + clear_ref_array(): rename from free_ref_array()\n + refs: rename parameters result -> sha1\n + refs: rename \"refname\" variables\n + struct ref_entry: document name member\n + cache.h: add comments for git_path() and git_path_submodule()\n (this branch is tangled with mh/ref-api-2.)\n\nExpecting a re-roll; will eject from 'pu' so that the topioc can lose the\nhot-fix at the tip.\n"},{"id":"180340","messageId":"CALkWK0mpPoZJWviBesWgy2dZ4xJrNyhED2znFid8iGbSTirPhQ@mail.gmail.com","threadId":"29092","inReplyTo":"7v8vmqi98f.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-06T05:35:08Z","receivedAt":"2011-12-06T05:35:08Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano wrote:\n> * rr/revert-cherry-pick (2011-10-23) 5 commits\n>  (merged to 'next' on 2011-10-26 at 27b7496)\n> [...]\n>\n> The internals of \"git revert/cherry-pick\" has been further refactored to\n> serve as the basis for the sequencer.\n>\n> Is this ready for 'master', or should we eject it to 'pu', expecing a\n> re-roll?\n\n[Notation: `rr/sequencer` is a new series I'm working on based on\n`rr/revert-cherry-pick`]\n\nI'm confused and I need your advice.  I've stopped working on\n`rr/sequencer` because this branch is out-of-date now.  We can do one\nof the following the improve the situation:\n0. You can merge this into `master` resolving the conflicts: it's a\nfairly straightforward resolution.  As soon as you publish the new\n`master`, I can continue working on `rr/sequencer`.\n1. I can post the `rr/revert-cherry-pick` to the list, hoping that it\nwill make it to `master` without more disruptions.  I can rebase\n`rr/sequencer` on this new series and continue working.  For your\nreference, this [1] is what I'll be posting to the list if we pick\nthis option.\n2. I can squash `rr/sequencer` into `rr/revert-cherry-pick` to post an\nentirely new series.  I think this is a really bad option.\n\nThanks.\n\n-- Ram\n\n[1]: https://github.com/artagnon/git/commits/rr/revert-cherry-pick\n"},{"id":"180342","messageId":"20111206055239.GA20671@sigill.intra.peff.net","threadId":"29092","inReplyTo":"7v8vmqi98f.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-06T05:52:39Z","receivedAt":"2011-12-06T05:52:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 05, 2011 at 09:01:52PM -0800, Junio C Hamano wrote:\n\n> * jk/credentials (2011-11-28) 20 commits\n>  - fixup! 034c066e\n>  - compat/getpass: add a /dev/tty implementation\n>  - credential: use git_prompt instead of git_getpass\n>  - prompt: add PROMPT_ECHO flag\n>  - stub out getpass_echo function\n>  - refactor git_getpass into generic prompt function\n>  - move git_getpass to its own source file\n>  - t: add test harness for external credential helpers\n>  - credentials: add \"store\" helper\n>  - strbuf: add strbuf_add*_urlencode\n>  - credentials: add \"cache\" helper\n>  - docs: end-user documentation for the credential subsystem\n>  - credential: make relevance of http path configurable\n>  - credential: add credential.*.username\n>  - credential: apply helper config\n>  - http: use credential API to get passwords\n>  - credential: add function for parsing url components\n>  - introduce credentials API\n>  - t5550: fix typo\n>  - test-lib: add test_config_global variant\n> \n> Expecting a reroll?\n\nYes, I have a re-roll ready. I was holding back to see if some of the\nhelper authors might comment, but got nothing. Perhaps the new version\nwill spur some interest.\n\nAlso, let's drop the top git_getpass bits from the topic for now (they\nwill not be part of my rebase). They are a separate topic that can go on\ntop, but I think there was some question from Erik of whether we should\nsimply roll our own getpass().\n\n> * jk/upload-archive-use-start-command (2011-11-21) 1 commit\n>  - upload-archive: use start_command instead of fork\n> \n> What's the status of this one?\n\nI think what you have in pu is good, but of course I didn't actually\ntest it on Windows. Erik?\n\n> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit\n>  - archive: don't let remote clients get unreachable commits\n>  (this branch is used by jk/maint-upload-archive.)\n> \n> * jk/maint-upload-archive (2011-11-21) 1 commit\n>  - Merge branch 'jk/maint-1.6.2-upload-archive' into jk/maint-upload-archive\n>  (this branch uses jk/maint-1.6.2-upload-archive.)\n> \n> Will merge to 'next' after taking another look.\n\nThanks. I also have some followup patches to re-loosen to at least\ntrees reachable from refs. Do you want to leave the tightening to the\nmaint track, and then consider the re-loosening for master?\n\n-Peff\n"},{"id":"180367","messageId":"CACsJy8CZjrKpv53Ep8ietAHAeVW4bnYeXTa6x5FGncT1HXGWQg@mail.gmail.com","threadId":"29092","inReplyTo":"7v8vmqi98f.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-12-06T11:20:00Z","receivedAt":"2011-12-06T11:20:00Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 6, 2011 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Here are the topics that have been cooking.  Commits prefixed with '-' are\n> only in 'pu' (proposed updates) while commits prefixed with '+' are in\n> 'next'.\n\nWhat can I do to get \"build in repack\" [1] series in or moved forward?\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/185190\n-- \nDuy\n"},{"id":"180368","messageId":"CABPQNSbOReM71HaPmce3v_98NDu17fT3YnySR4pWzJEDa-RKnA@mail.gmail.com","threadId":"29092","inReplyTo":"20111206055239.GA20671@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-12-06T11:22:06Z","receivedAt":"2011-12-06T11:22:06Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 6, 2011 at 6:52 AM, Jeff King <peff@peff.net> wrote:\n> On Mon, Dec 05, 2011 at 09:01:52PM -0800, Junio C Hamano wrote:\n>\n>> * jk/credentials (2011-11-28) 20 commits\n>>  - fixup! 034c066e\n>>  - compat/getpass: add a /dev/tty implementation\n>>  - credential: use git_prompt instead of git_getpass\n>>  - prompt: add PROMPT_ECHO flag\n>>  - stub out getpass_echo function\n>>  - refactor git_getpass into generic prompt function\n>>  - move git_getpass to its own source file\n>>  - t: add test harness for external credential helpers\n>>  - credentials: add \"store\" helper\n>>  - strbuf: add strbuf_add*_urlencode\n>>  - credentials: add \"cache\" helper\n>>  - docs: end-user documentation for the credential subsystem\n>>  - credential: make relevance of http path configurable\n>>  - credential: add credential.*.username\n>>  - credential: apply helper config\n>>  - http: use credential API to get passwords\n>>  - credential: add function for parsing url components\n>>  - introduce credentials API\n>>  - t5550: fix typo\n>>  - test-lib: add test_config_global variant\n>>\n>> Expecting a reroll?\n>\n> Yes, I have a re-roll ready. I was holding back to see if some of the\n> helper authors might comment, but got nothing. Perhaps the new version\n> will spur some interest.\n>\n> Also, let's drop the top git_getpass bits from the topic for now (they\n> will not be part of my rebase). They are a separate topic that can go on\n> top, but I think there was some question from Erik of whether we should\n> simply roll our own getpass().\n>\n>> * jk/upload-archive-use-start-command (2011-11-21) 1 commit\n>>  - upload-archive: use start_command instead of fork\n>>\n>> What's the status of this one?\n>\n> I think what you have in pu is good, but of course I didn't actually\n> test it on Windows. Erik?\n>\n\nI tried to check out ee27ca4 and build, and got hit by a wall of warnings:\n\ncompat/mingw.h: In function 'waitpid':\ncompat/mingw.h:117: warning: pointer targets in passing argument 1 of\n'_cwait' differ in signedness\nd:\\msysgit\\mingw\\bin\\../lib/gcc/mingw32/4.4.0/../../../../include/process.h:60:\nnote: expected 'int *' but argument is of type 'unsigned int *'\n\nThis seems to be an issue that has been around since the birth of the\nwindows port, and can be fixed by making our waitpid signature\nidentical to what POSIX expects\n(http://pubs.opengroup.org/onlinepubs/7908799/xsh/waitpid.html):\n\n---8<---\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex a255898..2036013 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -111,7 +111,7 @@ static inline int mingw_unlink(const char *pathname)\n }\n #define unlink mingw_unlink\n\n-static inline int waitpid(pid_t pid, unsigned *status, unsigned options)\n+static inline int waitpid(pid_t pid, int *status, int options)\n {\n \tif (options == 0)\n \t\treturn _cwait(status, pid, 0);\n\n---8<---\n\nBut then, there's a couple of much more scary warnings:\n\ndecorate.c: In function 'hash_obj':\ndecorate.c:11: warning: dereferencing type-punned pointer will break\nstrict-aliasing rules\n<snip>\nobject.c: In function 'hash_obj':\nobject.c:48: warning: dereferencing type-punned pointer will break\nstrict-aliasing rules\n<snip>\nbuiltin-merge-recursive.c: In function 'cmd_merge_recursive':\nbuiltin-merge-recursive.c:49: warning: unknown conversion type character 'z' in\nformat\nbuiltin-merge-recursive.c:49: warning: format '%s' expects type 'char *', but ar\ngument 2 has type 'unsigned int'\nbuiltin-merge-recursive.c:49: warning: too many arguments for format\n\nLooking at the code, the first two looks like the real thing, and not\njust false positives.\n\nIIRC, the strict aliasing rule is not a part of C89, but is in C99.\nAnd GCC starts to generate code that depends on strict-aliasing rules\nwith -O3. We don't use -O3, so this might be theoretical, at least on\nmy machine at this time.\n\nThe last one is a usage if \"%zu\", which our printf doesn't support.\nThis comes from commit 73118f89 (\"merge-recursive.c: Add more generic\nmerge_recursive_generic()\"), and should probably be fixed by doing:\n---8<---\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 703045b..02242cc 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -45,8 +45,9 @@ int cmd_merge_recursive(int argc, const char **argv,\nconst char *prefix)\n \t\t\tbases[bases_count++] = sha;\n \t\t}\n \t\telse\n-\t\t\twarning(\"Cannot handle more than %zu bases. \"\n-\t\t\t\t\"Ignoring %s.\", ARRAY_SIZE(bases)-1, argv[i]);\n+\t\t\twarning(\"Cannot handle more than %\"PRIuMAX\" bases. \"\n+\t\t\t\t\"Ignoring %s.\",\n+\t\t\t\t(uintmax_t)ARRAY_SIZE(bases)-1, argv[i]);\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n \t\tdie(\"Not handling anything other than two heads merge.\");\n---8<---\n\nBut no matter what, something has clearly happened to our warning\nlevel, and that should probably be investigated.\n\nBack to topic: after fixing these (apart from the strict aliasing\nissues), t5000-tar-tree pass fine here. So I guess that's at least\nsomething. I haven't tested from git-daemon yet, but I don't have time\nfor that right now...\n\nI'll submit the first fix later tonight, as it's clearly an\nimprovement IMO... but perhaps it's better if Stephan just squash in\nthe latter in the next round of the series, since it seems to be in a\npu-only series so far?\n"},{"id":"180373","messageId":"CACsJy8AmwQxcKz9vhtvFJPPKpXeipufD_0OqoMv41SHQFZgqeQ@mail.gmail.com","threadId":"29092","inReplyTo":"7v8vmqi98f.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-12-06T14:01:23Z","receivedAt":"2011-12-06T14:01:23Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Dec 6, 2011 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> * nd/resolve-ref (2011-12-05) 2 commits\n>  (merged to 'next' on 2011-12-05 at cc79e86)\n>  + Copy resolve_ref() return value for longer use\n>  + Convert many resolve_ref() calls to read_ref*() and ref_exists()\n>\n> Will merge to 'master'.\n\nI thought this would be replaced by a new version [1] I posted a while\nago (and forgot to address comments). Do you want me to rebase that on\ntop of this or replace it?\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/185580\n-- \nDuy\n"},{"id":"180390","messageId":"7vliqph8a6.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":"CALkWK0mpPoZJWviBesWgy2dZ4xJrNyhED2znFid8iGbSTirPhQ@mail.gmail.com","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T18:20:01Z","receivedAt":"2011-12-06T18:20:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> 0. You can merge this into `master` resolving the conflicts: it's a\n> fairly straightforward resolution.  As soon as you publish the new\n> `master`, I can continue working on `rr/sequencer`.\n> 1. I can post the `rr/revert-cherry-pick` to the list, hoping that it\n> will make it to `master` without more disruptions.  I can rebase\n> `rr/sequencer` on this new series and continue working.  For your\n> reference, this [1] is what I'll be posting to the list if we pick\n> this option.\n\nMerging\n\n    https://github.com/artagnon/git.git rr/revert-cherry-pick ;# 1b7c2632\n\nand merging rr/revert-cherry-pick currently in 'pu' (6a156fd) to 'master'\nresults in identical trees, so I think these amount to the same thing.\n\nA few comments and thoughts, all minor.\n\n * On \"revert: simplify getting commit subject in format_todo()\"\n\n   The old code used to use get_message() on cur->item, checked its return\n   value (if cur->item is not parsed, or has already been used and its buffer\n   cleared, cur->item->buffer would be NULL and get_message() returns -1) and\n   issued an error. The new code uses find_commit_subject without first\n   checking if cur->item->buffer is NULL.\n\n   Does this mean the old code was overly defensive, or is the new code too\n   lax?\n\n   I understand that parse_insn_line() uses lookup_commit_reference() which\n   calls parse_object() on the object name (and if it is a tag, deref_tag()\n   will parse the tagged object until we see something that is not a tag), so\n   we know cur->item is parsed before we see it, so I suspect you merely were\n   being overly defensive, but I may be missing something.\n\n * On \"revert: make commit subjects in insn sheet optional\"\n\n   After finding the verb and advancing the pointer \"bol\" at the beginning of\n   the object name, end_of_object_name variable points at the first SP or LF\n   using strcspn(bol, \" \\n\"), but I wonder why we are not grabbing a run of\n   hexdigits instead, i.e. strspn(bol, \"0123456789abcdef\"). Is this so that\n   we can allow something like \"pick rr/revert-cherry-pick~3\"?\n\n   I also wonder if this should be (sorry for pcre) \"(pick|revert)\\s+(\\S+)\\s\"\n   instead, i.e. allow users with fat fingers to use one or more SP or even HT\n   to separate the verb and the operand.\n\n   The last test you added to 3510 in this patch runs test_line_count\n   unconditionally, by the way.\n\n * On \"revert: allow mixed pick and revert instructions\"\n\n   Reporting what we did not understand from parse_insn_line() is a good\n   idea, but I think the line number should be reported at the beginning\n   of the same line.\n\nI'd say that I'd prefer queuing re-rolled patches posted on the list; I'll\ndiscard the rr/revert-cherry-pick (6a156fd) from my tree.\n\nThanks.\n"},{"id":"180391","messageId":"7vhb1dh7ki.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":"20111206055239.GA20671@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T18:35:25Z","receivedAt":"2011-12-06T18:35:25Z","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> Also, let's drop the top git_getpass bits from the topic for now (they\n> will not be part of my rebase). They are a separate topic that can go on\n> top, but I think there was some question from Erik of whether we should\n> simply roll our own getpass().\n\nSounds sensible.\n\nI suspect that there may be a codepath where we could ask both username\nand password; instead of making two consecutive calls to getpass() or\ngit_prompt(), the series may want to give a higher level abstraction, so\nthat GUI can show a dialog with two input fields (single-line input and\npassword input) and interact only once with the user. Such an input widget\ncould _show_ the username, and optionally even let it edited (there may be\nramifications depending on how the codepath uses the username), while\nasking for the corresponding password.\n\n>> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit\n>>  - archive: don't let remote clients get unreachable commits\n>>  (this branch is used by jk/maint-upload-archive.)\n>> \n>> * jk/maint-upload-archive (2011-11-21) 1 commit\n>>  - Merge branch 'jk/maint-1.6.2-upload-archive' into jk/maint-upload-archive\n>>  (this branch uses jk/maint-1.6.2-upload-archive.)\n>> \n>> Will merge to 'next' after taking another look.\n>\n> Thanks. I also have some followup patches to re-loosen to at least\n> trees reachable from refs. Do you want to leave the tightening to the\n> maint track, and then consider the re-loosening for master?\n\nI was planning to first have the really tight version graduate to 'master'\nand ship it in 1.7.9, while possibly merging that to 1.7.8.X series. If we\nhear complaints from real users in the meantime before or after such\nreleases, we could apply loosening patch on top of these topics and call\nthem \"regression fix\", but I have been assuming that nobody would have\nbeen using this backdoor for anything that really matters.\n"},{"id":"180392","messageId":"20111206184726.GA9492@sigill.intra.peff.net","threadId":"29092","inReplyTo":"7vhb1dh7ki.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-06T18:47:26Z","receivedAt":"2011-12-06T18:47:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 06, 2011 at 10:35:25AM -0800, Junio C Hamano wrote:\n\n> > Also, let's drop the top git_getpass bits from the topic for now (they\n> > will not be part of my rebase). They are a separate topic that can go on\n> > top, but I think there was some question from Erik of whether we should\n> > simply roll our own getpass().\n> \n> Sounds sensible.\n> \n> I suspect that there may be a codepath where we could ask both username\n> and password; instead of making two consecutive calls to getpass() or\n> git_prompt(), the series may want to give a higher level abstraction, so\n> that GUI can show a dialog with two input fields (single-line input and\n> password input) and interact only once with the user. Such an input widget\n> could _show_ the username, and optionally even let it edited (there may be\n> ramifications depending on how the codepath uses the username), while\n> asking for the corresponding password.\n\nYes, I've considered that, too. But I think the idea of a combined\nusername/password is part of the credential code, and the right\ncall chain is something like:\n\n  credential_fill\n    -> call helpers with \"get\"; return if it works\n    -> credential_getpass\n       -> call helpers with \"ask\" for combined GUI prompt\n       -> otherwise, use git_prompt\n          -> git_prompt(\"username\")\n          -> git_prompt(\"password\")\n\nSo the \"switch getpass to a generic prompt\" idea is separate from\nproviding that higher-level abstraction.\n\n> >> * jk/maint-1.6.2-upload-archive (2011-11-21) 1 commit\n> >>  - archive: don't let remote clients get unreachable commits\n> >>  (this branch is used by jk/maint-upload-archive.)\n> [...]\n> I was planning to first have the really tight version graduate to 'master'\n> and ship it in 1.7.9, while possibly merging that to 1.7.8.X series. If we\n> hear complaints from real users in the meantime before or after such\n> releases, we could apply loosening patch on top of these topics and call\n> them \"regression fix\", but I have been assuming that nobody would have\n> been using this backdoor for anything that really matters.\n\nOK. I'll hold back on the loosening then.\n\n-Peff\n"},{"id":"180393","messageId":"20111206185218.GB9492@sigill.intra.peff.net","threadId":"29092","inReplyTo":"CABPQNSbOReM71HaPmce3v_98NDu17fT3YnySR4pWzJEDa-RKnA@mail.gmail.com","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-06T18:52:18Z","receivedAt":"2011-12-06T18:52:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 06, 2011 at 12:22:06PM +0100, Erik Faye-Lund wrote:\n\n> >> * jk/upload-archive-use-start-command (2011-11-21) 1 commit\n> >>  - upload-archive: use start_command instead of fork\n> >>\n> >> What's the status of this one?\n> >\n> > I think what you have in pu is good, but of course I didn't actually\n> > test it on Windows. Erik?\n> >\n> \n> I tried to check out ee27ca4 and build, and got hit by a wall of warnings:\n\nI think you are on the wrong topic. ee27ca4 is the maint topic for\n\"don't let remote clients get unreachable commits\", and is based on the\nancient 1.6.2. Which is why you are getting all of those warnings.\n\nThe topic in question is jk/upload-archive-use-start-command, which is\nat 1bc01ef, and should be based on recent-ish master.\n\n-Peff\n"},{"id":"180396","messageId":"7vd3c1h5xu.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":"CACsJy8CZjrKpv53Ep8ietAHAeVW4bnYeXTa6x5FGncT1HXGWQg@mail.gmail.com","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T19:10:37Z","receivedAt":"2011-12-06T19:10:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> What can I do to get \"build in repack\" [1] series in or moved forward?\n\nResubmit to re-ignite interest in reviews, perhaps?\n\nAs long as it is done right at the implementation level, I do not think\nthere is anything controversial in the desired end result, iow it is not\nlike we need three people who want to have repack rewritten in C for it to\nhappen.\n\nIt may be tricky to get the flushing of old and new packfiles right,\nthough.\n\nUse of reprepare_packed_git() is prerequisite if you want to do it in a\nsingle process, but once you start using that API, it may not gain much\nperformance benefit to link the whole pack-objects logic in to the process\nover a much simpler and less error prone approach of just rewriting the\nshell script straight to a small C program that spawns pack-objects\nbinary.\n"},{"id":"180399","messageId":"7v8vmph5v7.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":"CACsJy8AmwQxcKz9vhtvFJPPKpXeipufD_0OqoMv41SHQFZgqeQ@mail.gmail.com","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-06T19:12:12Z","receivedAt":"2011-12-06T19:12:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> I thought this would be replaced by a new version [1] I posted a while\n> ago (and forgot to address comments). Do you want me to rebase that on\n> top of this or replace it?\n\nI think what was merged to 'master' is already sane. If you have updates,\ntreat them just like other new topics.\n\nThanks.\n"},{"id":"180404","messageId":"CALkWK0nNtvrLHxQv17jfrFQ=BcwLfgh7eE9X-nDCCYY0nsOskg@mail.gmail.com","threadId":"29092","inReplyTo":"7vliqph8a6.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-06T20:30:52Z","receivedAt":"2011-12-06T20:30:52Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Junio,\n\nJunio C Hamano wrote:\n> A few comments and thoughts, all minor.\n>\n>  * On \"revert: simplify getting commit subject in format_todo()\"\n>\n>   The old code used to use get_message() on cur->item, checked its return\n>   value (if cur->item is not parsed, or has already been used and its buffer\n>   cleared, cur->item->buffer would be NULL and get_message() returns -1) and\n>   issued an error. The new code uses find_commit_subject without first\n>   checking if cur->item->buffer is NULL.\n>\n>   Does this mean the old code was overly defensive, or is the new code too\n>   lax?\n>\n>   I understand that parse_insn_line() uses lookup_commit_reference() which\n>   calls parse_object() on the object name (and if it is a tag, deref_tag()\n>   will parse the tagged object until we see something that is not a tag), so\n>   we know cur->item is parsed before we see it, so I suspect you merely were\n>   being overly defensive, but I may be missing something.\n\nRight.  Actually, I was being overly defensive because I was being\nlazy about having to deal the an empty-commit-subject case in the\nparser.  With \"revert: make commit subjects in insn sheet optional\",\nthat's no more a concern- I'll reorder these two patches, and explain\nthis detail in the commit message in the re-roll.\n\n>  * On \"revert: make commit subjects in insn sheet optional\"\n>\n>   After finding the verb and advancing the pointer \"bol\" at the beginning of\n>   the object name, end_of_object_name variable points at the first SP or LF\n>   using strcspn(bol, \" \\n\"), but I wonder why we are not grabbing a run of\n>   hexdigits instead, i.e. strspn(bol, \"0123456789abcdef\"). Is this so that\n>   we can allow something like \"pick rr/revert-cherry-pick~3\"?\n\nYes, it is exactly for that reason :)\n\n>   I also wonder if this should be (sorry for pcre) \"(pick|revert)\\s+(\\S+)\\s\"\n>   instead, i.e. allow users with fat fingers to use one or more SP or even HT\n>   to separate the verb and the operand.\n\nHm, I'm not too enthusiastic about this change, because we don't\nadvertise hand-editing the instruction sheet yet: it's just some extra\nparsing cruft along with guardian buffer-overflow code that buys us no\nimmediate benefits.\n\n>   The last test you added to 3510 in this patch runs test_line_count\n>   unconditionally, by the way.\n\nGood catch.  Missing the chaining '&&' seems to be quite a common\nerror: I vaguely remember seeing a patch to detect this sometime ago.\nJonathan?\n\n>  * On \"revert: allow mixed pick and revert instructions\"\n>\n>   Reporting what we did not understand from parse_insn_line() is a good\n>   idea, but I think the line number should be reported at the beginning\n>   of the same line.\n\nMakes sense.  Do you like this?\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex e447449..cfa770e 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -715,7 +715,8 @@ static int format_todo(struct strbuf *buf, struct\nreplay_insn_list *todo_list)\n \treturn 0;\n }\n\n-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)\n+static int parse_insn_line(char *bol, char *eol,\n+\t\t\tstruct replay_insn_list *item, int lineno)\n {\n \tunsigned char commit_sha1[20];\n \tchar *end_of_object_name;\n@@ -731,7 +732,8 @@ static int parse_insn_line(char *bol, char *eol,\nstruct replay_insn_list *item)\n \t\tsize_t len = strchrnul(bol, '\\n') - bol;\n \t\tif (len > 255)\n \t\t\tlen = 255;\n-\t\treturn error(_(\"Unrecognized action: %.*s\"), (int)len, bol);\n+\t\treturn error(_(\"Line %d: Unrecognized action: %.*s\"),\n+\t\t\tlineno, (int)len, bol);\n \t}\n\n \tend_of_object_name = bol + strcspn(bol, \" \\n\");\n@@ -741,11 +743,11 @@ static int parse_insn_line(char *bol, char *eol,\nstruct replay_insn_list *item)\n \t*end_of_object_name = saved;\n\n \tif (status < 0)\n-\t\treturn error(_(\"Malformed object name: %s\"), bol);\n+\t\treturn error(_(\"Line %d: Malformed object name: %s\"), lineno, bol);\n\n \titem->operand = lookup_commit_reference(commit_sha1);\n \tif (!item->operand)\n-\t\treturn error(_(\"Not a valid commit: %s\"), bol);\n+\t\treturn error(_(\"Line %d: Not a valid commit: %s\"), lineno, bol);\n\n \titem->next = NULL;\n \treturn 0;\n@@ -760,8 +762,8 @@ static int parse_insn_buffer(char *buf, struct\nreplay_insn_list **todo_list)\n\n \tfor (i = 1; *p; i++) {\n \t\tchar *eol = strchrnul(p, '\\n');\n-\t\tif (parse_insn_line(p, eol, &item) < 0)\n-\t\t\treturn error(_(\"on line %d.\"), i);\n+\t\tif (parse_insn_line(p, eol, &item, lineno) < 0)\n+\t\t\treturn -1;\n \t\tnext = replay_insn_list_append(item.action, item.operand, next);\n \t\tp = *eol ? eol + 1 : eol;\n \t}\n--\n\n> I'd say that I'd prefer queuing re-rolled patches posted on the list; I'll\n> discard the rr/revert-cherry-pick (6a156fd) from my tree.\n\nThanks for letting me know -- will post shortly.\n\n-- Ram\n"},{"id":"180410","messageId":"4EDE8531.8090406@diamand.org","threadId":"29092","inReplyTo":"7v8vmqi98f.fsf@alter.siamese.dyndns.org","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-12-06T21:12:17Z","receivedAt":"2011-12-06T21:12:17Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 06/12/11 05:01, Junio C Hamano wrote:\n> Here are the topics that have been cooking.  Commits prefixed with '-' are\n> only in 'pu' (proposed updates) while commits prefixed with '+' are in\n> 'next'.\n>\n\n\n>\n> Will merge to 'next'.\n>\n> * ld/p4-labels-branches (2011-11-30) 4 commits\n>   - git-p4: importing labels should cope with missing owner\n>   - git-p4: add test for p4 labels\n>   - git-p4: cope with labels with empty descriptions\n>   - git-p4: handle p4 branches and labels containing shell chars\n>\n> I understand this has been retracted---please correct me otherwise.\n> Will discard, expecting a reroll.\n\nYes, discard this one and I'll re-roll it with fixes for the other label \nissues.\n\nThanks\nLuke\n"},{"id":"180477","messageId":"20111207100858.GB13374@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"CALkWK0nNtvrLHxQv17jfrFQ=BcwLfgh7eE9X-nDCCYY0nsOskg@mail.gmail.com","subject":"&&-chaining tester","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T10:08:58Z","receivedAt":"2011-12-07T10:08:58Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Missing the chaining '&&' seems to be quite a common\n> error: I vaguely remember seeing a patch to detect this sometime ago.\n> Jonathan?\n\nI guess you mean [1].\n\nIf you want to deploy it, you have three choices:\n\n - maintain a list of tests known to have broken &&-chaining, and\n   set GIT_SKIP_TESTS to avoid them\n\n - fix all cases of broken &&-chaining.  For extra bonus points,\n   send out those patches, respond to reviews, and gently usher\n   them into Junio's tree\n\n - modify the &&-chaining tester to output a list of tests encountered\n   with broken &&-chaining instead of halting on the first one.\n\nSome day, I'd like to (help) carry out option (b), but please don't\nhold your breath.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=158265\n"},{"id":"180505","messageId":"1323286611-4806-1-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"20111207100858.GB13374@elie.hsd1.il.comcast.net","subject":"Re: &&-chaining tester","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:36Z","receivedAt":"2011-12-07T19:36:36Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> Missing the chaining '&&' seems to be quite a common\n>> error: I vaguely remember seeing a patch to detect this sometime ago.\n>> Jonathan?\n>\n> [...]\n>  - fix all cases of broken &&-chaining.  For extra bonus points,\n>   send out those patches, respond to reviews, and gently usher\n>   them into Junio's tree\n\n\nHm, involves a huge amount of janitorial work.  Anyway, thanks for\npointing me in the right direction- here's a small start.\n\n[What I used; for reference]\n--\n t/test-lib.sh |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex bdd9513..3bf75ac 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -472,7 +472,17 @@ test_eval_ () {\n \teval >&3 2>&4 \"$*\"\n }\n \n+check_command_chaining_ () {\n+\teval >&3 2>&4 \"(exit 189) && $1\"\n+\teval_chain_ret=$?\n+\tif test \"$eval_chain_ret\" != 189\n+\tthen\n+\t\techo '[CHAIN_BUG] in the test script: missing \"&&\" in test commands'\n+\tfi\n+}\n+\n test_run_ () {\n+\tcheck_command_chaining_ \"$1\"\n \ttest_cleanup=:\n \texpecting_failure=$2\n \ttest_eval_ \"$1\"\n-- \n\nThanks.\n\n-- Ram\n\nRamkumar Ramachandra (15):\n  t1013 (loose-object-format): fix && chaining\n  t1300 (repo-config): fix && chaining\n  t1412 (reflog-loop): fix && chaining\n  t1007 (hash-object): fix && chaining\n  t1510 (repo-setup): fix && chaining\n  t1511 (rev-parse-caret): fix && chaining\n  t1510 (worktree): fix && chaining\n  t3200 (branch): fix && chaining\n  t3418 (rebase-continue): fix && chaining\n  t3400 (rebase): fix && chaining\n  t3310 (notes-merge-manual-resolve): fix && chaining\n  t3419 (rebase-patch-id): fix && chaining\n  t3030 (merge-recursive): use test_expect_code\n  t1006 (cat-file): use test_cmp\n  t3040 (subprojects-basic): modernize style\n\n t/t1006-cat-file.sh                   |   53 +++---------\n t/t1007-hash-object.sh                |   16 ++--\n t/t1013-loose-object-format.sh        |    2 +-\n t/t1300-repo-config.sh                |    2 +-\n t/t1412-reflog-loop.sh                |    2 +-\n t/t1501-worktree.sh                   |    6 +-\n t/t1510-repo-setup.sh                 |    4 +-\n t/t1511-rev-parse-caret.sh            |    2 +-\n t/t3030-merge-recursive.sh            |   72 ++---------------\n t/t3040-subprojects-basic.sh          |  144 ++++++++++++++++----------------\n t/t3200-branch.sh                     |    4 +-\n t/t3310-notes-merge-manual-resolve.sh |   10 +-\n t/t3400-rebase.sh                     |    4 +-\n t/t3418-rebase-continue.sh            |    4 +-\n t/t3419-rebase-patch-id.sh            |    2 +-\n 15 files changed, 119 insertions(+), 208 deletions(-)\n\n-- \n1.7.7.3\n"},{"id":"180507","messageId":"1323286611-4806-2-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 01/15] t1013 (loose-object-format): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:37Z","receivedAt":"2011-12-07T19:36:37Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1013-loose-object-format.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t1013-loose-object-format.sh b/t/t1013-loose-object-format.sh\nindex 0a9cedd..fbf5f2f 100755\n--- a/t/t1013-loose-object-format.sh\n+++ b/t/t1013-loose-object-format.sh\n@@ -34,7 +34,7 @@ assert_blob_equals() {\n }\n \n test_expect_success setup '\n-\tcp -R \"$TEST_DIRECTORY/t1013/objects\" .git/\n+\tcp -R \"$TEST_DIRECTORY/t1013/objects\" .git/ &&\n \tgit --version\n '\n \n-- \n1.7.7.3\n"},{"id":"180506","messageId":"1323286611-4806-3-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 02/15] t1300 (repo-config): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:38Z","receivedAt":"2011-12-07T19:36:38Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1300-repo-config.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 51caff0..0690e0e 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -38,7 +38,7 @@ cat > expect << EOF\n \tWhatEver = Second\n EOF\n test_expect_success 'similar section' '\n-\tgit config Cores.WhatEver Second\n+\tgit config Cores.WhatEver Second &&\n \ttest_cmp expect .git/config\n '\n \n-- \n1.7.7.3\n"},{"id":"180508","messageId":"1323286611-4806-4-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 03/15] t1412 (reflog-loop): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:39Z","receivedAt":"2011-12-07T19:36:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1412-reflog-loop.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t1412-reflog-loop.sh b/t/t1412-reflog-loop.sh\nindex 647d888..3acd895 100755\n--- a/t/t1412-reflog-loop.sh\n+++ b/t/t1412-reflog-loop.sh\n@@ -20,7 +20,7 @@ test_expect_success 'setup reflog with alternating commits' '\n '\n \n test_expect_success 'reflog shows all entries' '\n-\tcat >expect <<-\\EOF\n+\tcat >expect <<-\\EOF &&\n \t\ttopic@{0} reset: moving to two\n \t\ttopic@{1} reset: moving to one\n \t\ttopic@{2} reset: moving to two\n-- \n1.7.7.3\n"},{"id":"180509","messageId":"1323286611-4806-5-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 04/15] t1007 (hash-object): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:40Z","receivedAt":"2011-12-07T19:36:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1007-hash-object.sh |   16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh\nindex 6d52b82..316c60a 100755\n--- a/t/t1007-hash-object.sh\n+++ b/t/t1007-hash-object.sh\n@@ -154,13 +154,13 @@ test_expect_success 'check that --no-filters option works with --stdin-paths' '\n pop_repo\n \n for args in \"-w --stdin\" \"--stdin -w\"; do\n-\tpush_repo\n+\tpush_repo &&\n \n \ttest_expect_success \"hash from stdin and write to database ($args)\" '\n \t\ttest $example_sha1 = $(git hash-object $args < example)\n-\t'\n+\t' &&\n \n-\ttest_blob_exists $example_sha1\n+\ttest_blob_exists $example_sha1 &&\n \n \tpop_repo\n done\n@@ -176,20 +176,20 @@ test_expect_success \"hash two files with names on stdin\" '\n '\n \n for args in \"-w --stdin-paths\" \"--stdin-paths -w\"; do\n-\tpush_repo\n+\tpush_repo &&\n \n \ttest_expect_success \"hash two files with names on stdin and write to database ($args)\" '\n \t\ttest \"$sha1s\" = \"$(echo_without_newline \"$filenames\" | git hash-object $args)\"\n-\t'\n+\t' &&\n \n-\ttest_blob_exists $hello_sha1\n-\ttest_blob_exists $example_sha1\n+\ttest_blob_exists $hello_sha1 &&\n+\ttest_blob_exists $example_sha1 &&\n \n \tpop_repo\n done\n \n test_expect_success 'corrupt tree' '\n-\techo abc >malformed-tree\n+\techo abc >malformed-tree &&\n \ttest_must_fail git hash-object -t tree malformed-tree\n '\n \n-- \n1.7.7.3\n"},{"id":"180510","messageId":"1323286611-4806-6-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 05/15] t1510 (repo-setup): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:41Z","receivedAt":"2011-12-07T19:36:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1510-repo-setup.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex ec50a9a..80aedfc 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -603,7 +603,7 @@ test_expect_success '#22a: core.worktree = GIT_DIR = .git dir' '\n \t# like case #6.\n \n \tsetup_repo 22a \"$here/22a/.git\" \"\" unset &&\n-\tsetup_repo 22ab . \"\" unset\n+\tsetup_repo 22ab . \"\" unset &&\n \tmkdir -p 22a/.git/sub 22a/sub &&\n \tmkdir -p 22ab/.git/sub 22ab/sub &&\n \ttry_case 22a/.git unset . \\\n@@ -742,7 +742,7 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '\n # Case #29: GIT_WORK_TREE(+core.worktree) overrides core.bare (gitfile case).\n test_expect_success '#29: setup' '\n \tsetup_repo 29 non-existent gitfile true &&\n-\tmkdir -p 29/sub/sub 29/wt/sub\n+\tmkdir -p 29/sub/sub 29/wt/sub &&\n \t(\n \t\tcd 29 &&\n \t\tGIT_WORK_TREE=\"$here/29\" &&\n-- \n1.7.7.3\n"},{"id":"180511","messageId":"1323286611-4806-7-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 06/15] t1511 (rev-parse-caret): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:42Z","receivedAt":"2011-12-07T19:36:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1511-rev-parse-caret.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh\nindex e043cb7..eaefc77 100755\n--- a/t/t1511-rev-parse-caret.sh\n+++ b/t/t1511-rev-parse-caret.sh\n@@ -6,7 +6,7 @@ test_description='tests for ref^{stuff}'\n \n test_expect_success 'setup' '\n \techo blob >a-blob &&\n-\tgit tag -a -m blob blob-tag `git hash-object -w a-blob`\n+\tgit tag -a -m blob blob-tag `git hash-object -w a-blob` &&\n \tmkdir a-tree &&\n \techo moreblobs >a-tree/another-blob &&\n \tgit add . &&\n-- \n1.7.7.3\n"},{"id":"180512","messageId":"1323286611-4806-8-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 07/15] t1510 (worktree): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:43Z","receivedAt":"2011-12-07T19:36:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1501-worktree.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh\nindex 6384983..d9761bd 100755\n--- a/t/t1501-worktree.sh\n+++ b/t/t1501-worktree.sh\n@@ -48,7 +48,7 @@ test_expect_success 'setup: helper for testing rev-parse' '\n '\n \n test_expect_success 'setup: core.worktree = relative path' '\n-\tunset GIT_WORK_TREE;\n+\tunset GIT_WORK_TREE &&\n \tGIT_DIR=repo.git &&\n \tGIT_CONFIG=\"$(pwd)\"/$GIT_DIR/config &&\n \texport GIT_DIR GIT_CONFIG &&\n@@ -89,7 +89,7 @@ test_expect_success 'subdir of work tree' '\n '\n \n test_expect_success 'setup: core.worktree = absolute path' '\n-\tunset GIT_WORK_TREE;\n+\tunset GIT_WORK_TREE &&\n \tGIT_DIR=$(pwd)/repo.git &&\n \tGIT_CONFIG=$GIT_DIR/config &&\n \texport GIT_DIR GIT_CONFIG &&\n@@ -334,7 +334,7 @@ test_expect_success 'absolute pathspec should fail gracefully' '\n '\n \n test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '\n-\t>dummy_file\n+\t>dummy_file &&\n \techo git --git-dir=\"$(pwd)//repo.git\" --work-tree=\"$(pwd)\" add dummy_file &&\n \tgit --git-dir=\"$(pwd)//repo.git\" --work-tree=\"$(pwd)\" add dummy_file\n '\n-- \n1.7.7.3\n"},{"id":"180513","messageId":"1323286611-4806-9-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 08/15] t3200 (branch): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:44Z","receivedAt":"2011-12-07T19:36:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3200-branch.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex bc73c20..7877290 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -22,7 +22,7 @@ test_expect_success \\\n \n test_expect_success \\\n     'git branch --help should not have created a bogus branch' '\n-     git branch --help </dev/null >/dev/null 2>/dev/null;\n+     git branch --help </dev/null >/dev/null 2>/dev/null &&\n      test_path_is_missing .git/refs/heads/--help\n '\n \n@@ -88,7 +88,7 @@ test_expect_success \\\n test_expect_success \\\n     'git branch -m n/n n should work' \\\n        'git branch -l n/n &&\n-        git branch -m n/n n\n+        git branch -m n/n n &&\n \ttest_path_is_file .git/logs/refs/heads/n'\n \n test_expect_success 'git branch -m o/o o should fail when o/p exists' '\n-- \n1.7.7.3\n"},{"id":"180514","messageId":"1323286611-4806-10-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 09/15] t3418 (rebase-continue): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:45Z","receivedAt":"2011-12-07T19:36:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3418-rebase-continue.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 1e855cd..2680375 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -51,7 +51,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n \ttest_commit \"commit-new-file-F3-on-topic-branch\" F3 32 &&\n \ttest_when_finished \"rm -fr test-bin funny.was.run\" &&\n \tmkdir test-bin &&\n-\tcat >test-bin/git-merge-funny <<-EOF\n+\tcat >test-bin/git-merge-funny <<-EOF &&\n \t#!$SHELL_PATH\n \tcase \"\\$1\" in --opt) ;; *) exit 2 ;; esac\n \tshift &&\n@@ -77,7 +77,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '\n test_expect_success 'rebase --continue remembers --rerere-autoupdate' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F3-on-topic-branch &&\n-\tgit checkout master\n+\tgit checkout master &&\n \ttest_commit \"commit-new-file-F3\" F3 3 &&\n \tgit config rerere.enabled true &&\n \ttest_must_fail git rebase -m master topic &&\n-- \n1.7.7.3\n"},{"id":"180515","messageId":"1323286611-4806-11-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 10/15] t3400 (rebase): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:46Z","receivedAt":"2011-12-07T19:36:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3400-rebase.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 6eaecec..c355533 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -172,8 +172,8 @@ test_expect_success 'fail when upstream arg is missing and not configured' '\n \n test_expect_success 'default to @{upstream} when upstream arg is missing' '\n \tgit checkout -b default topic &&\n-\tgit config branch.default.remote .\n-\tgit config branch.default.merge refs/heads/master\n+\tgit config branch.default.remote . &&\n+\tgit config branch.default.merge refs/heads/master &&\n \tgit rebase &&\n \ttest \"$(git rev-parse default~1)\" = \"$(git rev-parse master)\"\n '\n-- \n1.7.7.3\n"},{"id":"180517","messageId":"1323286611-4806-12-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 11/15] t3310 (notes-merge-manual-resolve): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:47Z","receivedAt":"2011-12-07T19:36:47Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3310-notes-merge-manual-resolve.sh |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh\nindex 4ec4d11..4367197 100755\n--- a/t/t3310-notes-merge-manual-resolve.sh\n+++ b/t/t3310-notes-merge-manual-resolve.sh\n@@ -389,7 +389,7 @@ test_expect_success 'abort notes merge' '\n \ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == y)\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(cat pre_merge_y)\" &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\n@@ -525,9 +525,9 @@ EOF\n \ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&\n \ttest -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&\n \t# Refs are unchanged\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n-\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\"\n-\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n+\ttest \"$(git rev-parse refs/notes/y)\" = \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n+\ttest \"$(git rev-parse refs/notes/m)\" != \"$(git rev-parse NOTES_MERGE_PARTIAL^1)\" &&\n \t# Mention refs/notes/m, and its current and expected value in output\n \tgrep -q \"refs/notes/m\" output &&\n \tgrep -q \"$(git rev-parse refs/notes/m)\" output &&\n@@ -545,7 +545,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '\n \ttest_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&\n \ttest_cmp /dev/null output &&\n \t# m has not moved (still == w)\n-\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\"\n+\ttest \"$(git rev-parse refs/notes/m)\" = \"$(git rev-parse refs/notes/w)\" &&\n \t# Verify that other notes refs has not changed (w, x, y and z)\n \tverify_notes w &&\n \tverify_notes x &&\n-- \n1.7.7.3\n"},{"id":"180516","messageId":"1323286611-4806-13-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 12/15] t3419 (rebase-patch-id): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:48Z","receivedAt":"2011-12-07T19:36:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3419-rebase-patch-id.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh\nindex bd8efaf..e70ac10 100755\n--- a/t/t3419-rebase-patch-id.sh\n+++ b/t/t3419-rebase-patch-id.sh\n@@ -39,7 +39,7 @@ run()\n }\n \n test_expect_success 'setup' '\n-\tgit commit --allow-empty -m initial\n+\tgit commit --allow-empty -m initial &&\n \tgit tag root\n '\n \n-- \n1.7.7.3\n"},{"id":"180518","messageId":"1323286611-4806-14-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 13/15] t3030 (merge-recursive): use test_expect_code","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:49Z","receivedAt":"2011-12-07T19:36:49Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Use test_expect_code in preference to repeatedly checking exit codes\nby hand.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3030-merge-recursive.sh |   72 ++++----------------------------------------\n 1 files changed, 6 insertions(+), 66 deletions(-)\n\ndiff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh\nindex 55ef189..a5e3da7 100755\n--- a/t/t3030-merge-recursive.sh\n+++ b/t/t3030-merge-recursive.sh\n@@ -285,17 +285,7 @@ test_expect_success 'merge-recursive simple' '\n \trm -fr [abcd] &&\n \tgit checkout -f \"$c2\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive result' '\n@@ -334,17 +324,7 @@ test_expect_success 'merge-recursive remove conflict' '\n \trm -fr [abcd] &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c5\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c5\"\n '\n \n test_expect_success 'merge-recursive remove conflict' '\n@@ -388,17 +368,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c4\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c4\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n@@ -422,17 +392,7 @@ test_expect_success 'merge-recursive d/f conflict the other way' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c4\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c4\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c4\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result the other way' '\n@@ -456,17 +416,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c1\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c1\" \"$c6\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c1\" \"$c6\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n@@ -490,17 +440,7 @@ test_expect_success 'merge-recursive d/f conflict' '\n \tgit reset --hard &&\n \tgit checkout -f \"$c6\" &&\n \n-\tgit merge-recursive \"$c0\" -- \"$c6\" \"$c1\"\n-\tstatus=$?\n-\tcase \"$status\" in\n-\t1)\n-\t\t: happy\n-\t\t;;\n-\t*)\n-\t\techo >&2 \"why status $status!!!\"\n-\t\tfalse\n-\t\t;;\n-\tesac\n+\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c6\" \"$c1\"\n '\n \n test_expect_success 'merge-recursive d/f conflict result' '\n-- \n1.7.7.3\n"},{"id":"180519","messageId":"1323286611-4806-15-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 14/15] t1006 (cat-file): use test_cmp","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:50Z","receivedAt":"2011-12-07T19:36:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Use test_cmp in preference to repeatedly comparing command outputs by\nhand.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1006-cat-file.sh |   53 +++++++++++---------------------------------------\n 1 files changed, 12 insertions(+), 41 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex d8b7f2f..af6cacc 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -45,57 +45,28 @@ $content\"\n \n     test -z \"$content\" ||\n     test_expect_success \"Content of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$content\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts)\"\n-\n-        if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n-        fi\n+\tmaybe_remove_timestamp \"$content\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(git cat-file $type $sha1)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n     '\n \n     test_expect_success \"Pretty content of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$pretty_content\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(git cat-file -p $sha1)\" $no_ts)\"\n-        if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n-        fi\n+\tmaybe_remove_timestamp \"$pretty_content\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(git cat-file -p $sha1)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n     '\n \n     test -z \"$content\" ||\n     test_expect_success \"--batch output of $type is correct\" '\n-\texpect=\"$(maybe_remove_timestamp \"$batch_output\" $no_ts)\"\n-\tactual=\"$(maybe_remove_timestamp \"$(echo $sha1 | git cat-file --batch)\" $no_ts)\"\n-        if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n-        fi\n+\tmaybe_remove_timestamp \"$batch_output\" $no_ts >expect &&\n+\tmaybe_remove_timestamp \"$(echo $sha1 | git cat-file --batch)\" $no_ts >actual &&\n+\ttest_cmp expect actual\n     '\n \n     test_expect_success \"--batch-check output of $type is correct\" '\n-\texpect=\"$sha1 $type $size\"\n-\tactual=\"$(echo_without_newline $sha1 | git cat-file --batch-check)\"\n-        if test \"z$expect\" = \"z$actual\"\n-\tthen\n-\t\t: happy\n-\telse\n-\t\techo \"Oops: expected $expect\"\n-\t\techo \"but got $actual\"\n-\t\tfalse\n-        fi\n+\techo \"$sha1 $type $size\" >expect &&\n+\techo_without_newline $sha1 | git cat-file --batch-check >actual &&\n+\ttest_cmp expect actual\n     '\n }\n \n-- \n1.7.7.3\n"},{"id":"180520","messageId":"1323286611-4806-16-git-send-email-artagnon@gmail.com","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 15/15] t3040 (subprojects-basic): modernize style","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-07T19:36:51Z","receivedAt":"2011-12-07T19:36:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Put the opening quote starting each test on the same line as the\ntest_expect_* invocation.  While at it:\n\n- Indent the file with tabs, not spaces.\n\n- Guard commands that prepare test input for individual tests in the\n  same test_expect_success, so that their scope is clearer and errors\n  at that stage can be caught.\n\n- Use <<\\-EOF in preference to <<EOF to save readers the trouble of\n  looking for variable interpolations.\n\n- Include \"setup\" in the titles of test assertions that prepare for\n  later ones to make it more obvious which tests can be skipped.\n\n- Chain commands with &&.  Breaks in a test assertion's && chain can\n  potentially hide failures from earlier commands in the chain.\n\n- Use test_expect_code() in preference to checking the exit status of\n  various statements by hand.\n\nInspired-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t3040-subprojects-basic.sh |  144 +++++++++++++++++++++---------------------\n 1 files changed, 72 insertions(+), 72 deletions(-)\n\ndiff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh\nindex f6973e9..0a4ff6d 100755\n--- a/t/t3040-subprojects-basic.sh\n+++ b/t/t3040-subprojects-basic.sh\n@@ -3,81 +3,81 @@\n test_description='Basic subproject functionality'\n . ./test-lib.sh\n \n-test_expect_success 'Super project creation' \\\n-    ': >Makefile &&\n-    git add Makefile &&\n-    git commit -m \"Superproject created\"'\n-\n-\n-cat >expected <<EOF\n-:000000 160000 00000... A\tsub1\n-:000000 160000 00000... A\tsub2\n-EOF\n-test_expect_success 'create subprojects' \\\n-    'mkdir sub1 &&\n-    ( cd sub1 && git init && : >Makefile && git add * &&\n-    git commit -q -m \"subproject 1\" ) &&\n-    mkdir sub2 &&\n-    ( cd sub2 && git init && : >Makefile && git add * &&\n-    git commit -q -m \"subproject 2\" ) &&\n-    git update-index --add sub1 &&\n-    git add sub2 &&\n-    git commit -q -m \"subprojects added\" &&\n-    git diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n-    test_cmp expected current'\n-\n-git branch save HEAD\n-\n-test_expect_success 'check if fsck ignores the subprojects' \\\n-    'git fsck --full'\n-\n-test_expect_success 'check if commit in a subproject detected' \\\n-    '( cd sub1 &&\n-    echo \"all:\" >>Makefile &&\n-    echo \"\ttrue\" >>Makefile &&\n-    git commit -q -a -m \"make all\" ) && {\n-        git diff-files --exit-code\n-\ttest $? = 1\n-    }'\n-\n-test_expect_success 'check if a changed subproject HEAD can be committed' \\\n-    'git commit -q -a -m \"sub1 changed\" && {\n-\tgit diff-tree --exit-code HEAD^ HEAD\n-\ttest $? = 1\n-    }'\n-\n-test_expect_success 'check if diff-index works for subproject elements' \\\n-    'git diff-index --exit-code --cached save -- sub1\n-    test $? = 1'\n-\n-test_expect_success 'check if diff-tree works for subproject elements' \\\n-    'git diff-tree --exit-code HEAD^ HEAD -- sub1\n-    test $? = 1'\n-\n-test_expect_success 'check if git diff works for subproject elements' \\\n-    'git diff --exit-code HEAD^ HEAD\n-    test $? = 1'\n-\n-test_expect_success 'check if clone works' \\\n-    'git ls-files -s >expected &&\n-    git clone -l -s . cloned &&\n-    ( cd cloned && git ls-files -s ) >current &&\n-    test_cmp expected current'\n-\n-test_expect_success 'removing and adding subproject' \\\n-    'git update-index --force-remove -- sub2 &&\n-    mv sub2 sub3 &&\n-    git add sub3 &&\n-    git commit -q -m \"renaming a subproject\" && {\n-\tgit diff -M --name-status --exit-code HEAD^ HEAD\n-\ttest $? = 1\n-    }'\n+test_expect_success 'setup: create superproject' '\n+\t: >Makefile &&\n+\tgit add Makefile &&\n+\tgit commit -m \"Superproject created\"\n+'\n+\n+test_expect_success 'setup: create subprojects' '\n+\tmkdir sub1 &&\n+\t( cd sub1 && git init && : >Makefile && git add * &&\n+\tgit commit -q -m \"subproject 1\" ) &&\n+\tmkdir sub2 &&\n+\t( cd sub2 && git init && : >Makefile && git add * &&\n+\tgit commit -q -m \"subproject 2\" ) &&\n+\tgit update-index --add sub1 &&\n+\tgit add sub2 &&\n+\tgit commit -q -m \"subprojects added\" &&\n+\tgit diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n+\tgit branch save HEAD &&\n+\tcat >expected <<-\\EOF &&\n+\t:000000 160000 00000... A\tsub1\n+\t:000000 160000 00000... A\tsub2\n+\tEOF\n+\ttest_cmp expected current\n+'\n+\n+test_expect_success 'check if fsck ignores the subprojects' '\n+\tgit fsck --full\n+'\n+\n+test_expect_success 'check if commit in a subproject detected' '\n+\t( cd sub1 &&\n+\techo \"all:\" >>Makefile &&\n+\techo \"\ttrue\" >>Makefile &&\n+\tgit commit -q -a -m \"make all\" ) &&\n+\ttest_expect_code 1 git diff-files --exit-code\n+'\n+\n+test_expect_success 'check if a changed subproject HEAD can be committed' '\n+\tgit commit -q -a -m \"sub1 changed\" &&\n+\ttest_expect_code 1 git diff-tree --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'check if diff-index works for subproject elements' '\n+\ttest_expect_code 1 git diff-index --exit-code --cached save -- sub1\n+'\n+\n+test_expect_success 'check if diff-tree works for subproject elements' '\n+\ttest_expect_code 1 git diff-tree --exit-code HEAD^ HEAD -- sub1\n+'\n+\n+test_expect_success 'check if git diff works for subproject elements' '\n+\ttest_expect_code 1 git diff --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'check if clone works' '\n+\tgit ls-files -s >expected &&\n+\tgit clone -l -s . cloned &&\n+\t( cd cloned && git ls-files -s ) >current &&\n+\ttest_cmp expected current\n+'\n+\n+test_expect_success 'removing and adding subproject' '\n+\tgit update-index --force-remove -- sub2 &&\n+\tmv sub2 sub3 &&\n+\tgit add sub3 &&\n+\tgit commit -q -m \"renaming a subproject\" &&\n+\ttest_expect_code 1 git diff -M --name-status --exit-code HEAD^ HEAD\n+'\n \n # the index must contain the object name the HEAD of the\n # subproject sub1 was at the point \"save\"\n-test_expect_success 'checkout in superproject' \\\n-    'git checkout save &&\n-    git diff-index --exit-code --raw --cached save -- sub1'\n+test_expect_success 'checkout in superproject' '\n+\tgit checkout save &&\n+\tgit diff-index --exit-code --raw --cached save -- sub1\n+'\n \n # just interesting what happened...\n # git diff --name-status -M save master\n-- \n1.7.7.3\n"},{"id":"180525","messageId":"20111207214716.GA2911@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"1323286611-4806-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 04/15] t1007 (hash-object): fix && chaining","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T21:47:16Z","receivedAt":"2011-12-07T21:47:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\nI think there's room for at least one line of description about why\none would want to do this.\n\nIMHO if the patches are only being sent to me, Junio, and the\nmailing list (and not cc-ed to different people), then there's no\nreason to split them up when they have the same topic.\n\nAside from that, patches 1-3 look good.  Now for this one:\n\n[...]\n> --- a/t/t1007-hash-object.sh\n> +++ b/t/t1007-hash-object.sh\n> @@ -154,13 +154,13 @@ test_expect_success 'check that --no-filters option works with --stdin-paths' '\n>  pop_repo\n>  \n>  for args in \"-w --stdin\" \"--stdin -w\"; do\n> -\tpush_repo\n> +\tpush_repo &&\n>  \n>  \ttest_expect_success \"hash from stdin and write to database ($args)\" '\n>  \t\ttest $example_sha1 = $(git hash-object $args < example)\n> -\t'\n> +\t' &&\n\nI don't see how this would have any effect.  Is it intended?\n"},{"id":"180527","messageId":"20111207215100.GB2911@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"1323286611-4806-8-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 07/15] t1510 (worktree): fix && chaining","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T21:51:00Z","receivedAt":"2011-12-07T21:51:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Patches 5 and 6 look safe, though I haven't tested them.\n\nRamkumar Ramachandra wrote:\n\n> --- a/t/t1501-worktree.sh\n> +++ b/t/t1501-worktree.sh\n> @@ -48,7 +48,7 @@ test_expect_success 'setup: helper for testing rev-parse' '\n>  '\n>  \n>  test_expect_success 'setup: core.worktree = relative path' '\n> -\tunset GIT_WORK_TREE;\n> +\tunset GIT_WORK_TREE &&\n\nOn some shells, like /usr/xpg4/bin/sh on Solaris, unset returns nonzero\nstatus when the variable passed was already unset.  Will this work on\nsuch platforms, or does it need to be changed to use sane_unset?\n"},{"id":"180528","messageId":"20111207215509.GC2911@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"1323286611-4806-9-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 08/15] t3200 (branch): fix && chaining","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T21:55:09Z","receivedAt":"2011-12-07T21:55:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -22,7 +22,7 @@ test_expect_success \\\n>  \n>  test_expect_success \\\n>      'git branch --help should not have created a bogus branch' '\n> -     git branch --help </dev/null >/dev/null 2>/dev/null;\n> +     git branch --help </dev/null >/dev/null 2>/dev/null &&\n>       test_path_is_missing .git/refs/heads/--help\n\nWon't this break when running tests for the first time, before the git\nmanpages are installed?\n\n>  '\n>  \n> @@ -88,7 +88,7 @@ test_expect_success \\\n>  test_expect_success \\\n>      'git branch -m n/n n should work' \\\n>         'git branch -l n/n &&\n> -        git branch -m n/n n\n> +        git branch -m n/n n &&\n>  \ttest_path_is_file .git/logs/refs/heads/n'\n\nGood catch.\n"},{"id":"180530","messageId":"20111207215741.GD2911@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"1323286611-4806-14-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 13/15] t3030 (merge-recursive): use test_expect_code","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T21:57:42Z","receivedAt":"2011-12-07T21:57:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Patches 9-12 look good.\n\nRamkumar Ramachandra wrote:\n\n> Use test_expect_code in preference to repeatedly checking exit codes\n> by hand.\n[...]\n> +++ b/t/t3030-merge-recursive.sh\n> @@ -285,17 +285,7 @@ test_expect_success 'merge-recursive simple' '\n>  \trm -fr [abcd] &&\n>  \tgit checkout -f \"$c2\" &&\n>  \n> -\tgit merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n> -\tstatus=$?\n> -\tcase \"$status\" in\n> -\t1)\n> -\t\t: happy\n> -\t\t;;\n> -\t*)\n> -\t\techo >&2 \"why status $status!!!\"\n> -\t\tfalse\n> -\t\t;;\n> -\tesac\n> +\ttest_expect_code 1 git merge-recursive \"$c0\" -- \"$c2\" \"$c1\"\n\nYeah, this style is kind of repetitive.  Worse, if \"git checkout\"\nfails with status 1, it would yield a false success.  Thanks for\nfixing it.\n"},{"id":"180531","messageId":"20111207220127.GE2911@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"1323286611-4806-15-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 14/15] t1006 (cat-file): use test_cmp","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T22:01:27Z","receivedAt":"2011-12-07T22:01:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Use test_cmp in preference to repeatedly comparing command outputs by\n> hand.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  t/t1006-cat-file.sh |   53 +++++++++++---------------------------------------\n>  1 files changed, 12 insertions(+), 41 deletions(-)\n\nI guess test_cmp didn't exist yet when this code was written, though\nthat doesn't explain why \"diff\" or \"cmp\" was not used.\n\nDoesn't this miss some other instances of the\n\n\ttest $expect = $actual\n\nidiom in the same test script?  What distinguishes the cases that you\nfixed from the ones skipped, and is it worth an inconsistent style to\nconvert some without the others?\n"},{"id":"180535","messageId":"20111207222126.GF2911@elie.hsd1.il.comcast.net","threadId":"29092","inReplyTo":"1323286611-4806-16-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 15/15] t3040 (subprojects-basic): modernize style","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-07T22:21:26Z","receivedAt":"2011-12-07T22:21:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Put the opening quote starting each test on the same line as the\n> test_expect_* invocation.  While at it:\n[...]\n> - Use <<\\-EOF in preference to <<EOF to save readers the trouble of\n>   looking for variable interpolations.\n\nI think you mean <<-\\EOF. :)\n\n[...]\n> - Chain commands with &&.  Breaks in a test assertion's && chain can\n>   potentially hide failures from earlier commands in the chain.\n>\n> - Use test_expect_code() in preference to checking the exit status of\n>   various statements by hand.\n\nI guess these two are the motivation?\n\n> Inspired-by: Jonathan Nieder <jrnieder@gmail.com>\n\nOh, dear.\n\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n[...]\n> --- a/t/t3040-subprojects-basic.sh\n> +++ b/t/t3040-subprojects-basic.sh\n> @@ -3,81 +3,81 @@\n>  test_description='Basic subproject functionality'\n>  . ./test-lib.sh\n>  \n> -test_expect_success 'Super project creation' \\\n> -    ': >Makefile &&\n> -    git add Makefile &&\n> -    git commit -m \"Superproject created\"'\n> -\n> -\n> -cat >expected <<EOF\n[...]\n\nIt would be easier to read if each preimage test assertion were next to\nthe corresponding postimage test assertion.  Does \"git diff --patience\"\ndo better?\n\n> -test_expect_success 'Super project creation' \\\n> -    ': >Makefile &&\n> -    git add Makefile &&\n> -    git commit -m \"Superproject created\"'\n> -\n> -\n> +test_expect_success 'setup: create superproject' '\n> +\t: >Makefile &&\n> +\tgit add Makefile &&\n> +\tgit commit -m \"Superproject created\"\n> +'\n> +\n\nOk, makes sense.\n\n> -cat >expected <<EOF\n> -:000000 160000 00000... A\tsub1\n> -:000000 160000 00000... A\tsub2\n> -EOF\n> -test_expect_success 'create subprojects' \\\n> -    'mkdir sub1 &&\n> -    ( cd sub1 && git init && : >Makefile && git add * &&\n> -    git commit -q -m \"subproject 1\" ) &&\n> +test_expect_success 'setup: create subprojects' '\n> +\tmkdir sub1 &&\n> +\t( cd sub1 && git init && : >Makefile && git add * &&\n> +\tgit commit -q -m \"subproject 1\" ) &&\n\nIf cleaning up the style anyway, I would write this as\n\n\tmkdir sub1 &&\n\t(\n\t\tcd sub1 &&\n\t\tgit init &&\n\t\t>Makefile &&\n\t\tgit add Makefile &&\n\t\tgit commit -m \"subproject 1\"\n\t)\n\nOr\n\n\tmkdir sub1 &&\n\t(\n\t\tcd sub1 &&\n\t\tgit init &&\n\t\ttest_commit subproject-1 Makefile\n\t)\n\nBut leaving it alone like you did is probably better. ;-)\n\n[...]\n> -    git diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n> -    test_cmp expected current'\n> -\n> -git branch save HEAD\n> -\n> +\tgit diff-tree --abbrev=5 HEAD^ HEAD |cut -d\" \" -f-3,5- >current &&\n> +\tgit branch save HEAD &&\n> +\tcat >expected <<-\\EOF &&\n> +\t:000000 160000 00000... A\tsub1\n> +\t:000000 160000 00000... A\tsub2\n> +\tEOF\n> +\ttest_cmp expected current\n> +'\n> +\n\nAt first I wondered if \"git branch save HEAD\" is logically part of the\nsame test.  After all, it's just meant as a baseline for use by later\ntests.\n\nAfter thinking about it for a few seconds, though, that's exactly what\nthis test is about.  Maybe it would have been clearer if the two setup\ntests were combined into one (but please don't take this advice too\nseriously; I'm just musing).\n\n> -test_expect_success 'check if fsck ignores the subprojects' \\\n> -    'git fsck --full'\n> +test_expect_success 'check if fsck ignores the subprojects' '\n> +\tgit fsck --full\n> +'\n\nDoes this test imply that one of the subprojects is broken somehow?\n\n> -\n> -test_expect_success 'check if commit in a subproject detected' \\\n> -    '( cd sub1 &&\n> -    echo \"all:\" >>Makefile &&\n> -    echo \"\ttrue\" >>Makefile &&\n> -    git commit -q -a -m \"make all\" ) && {\n> -        git diff-files --exit-code\n> -\ttest $? = 1\n> -    }'\n> +\n> +test_expect_success 'check if commit in a subproject detected' '\n> +\t( cd sub1 &&\n> +\techo \"all:\" >>Makefile &&\n> +\techo \"\ttrue\" >>Makefile &&\n> +\tgit commit -q -a -m \"make all\" ) &&\n> +\ttest_expect_code 1 git diff-files --exit-code\n> +'\n\nNice.  Style again: I'd be tempted to reformat as\n\n\t(\n\t\tcd sub1 &&\n\t\techo \"all:\" >>Makefile &&\n\t\t...\n\t) &&\n\ttest_expect_code 1 git diff-files --exit-code\n\nto make the subshell scope a little clearer, but exercising restraint\nlike you did may be better.\n\n[...]\n>  \n>  # the index must contain the object name the HEAD of the\n>  # subproject sub1 was at the point \"save\"\n> -test_expect_success 'checkout in superproject' \\\n> -    'git checkout save &&\n> -    git diff-index --exit-code --raw --cached save -- sub1'\n> +test_expect_success 'checkout in superproject' '\n> +\tgit checkout save &&\n> +\tgit diff-index --exit-code --raw --cached save -- sub1\n> +'\n\nThanks much.  The result really is a little easier on the eyes, and\nthe changes look safe.\n"},{"id":"180554","messageId":"CALkWK0mQjJ_JeqK0h3cy++CKKdTzzsyOui9jO-kCriL0mUsCuQ@mail.gmail.com","threadId":"29092","inReplyTo":"20111207215100.GB2911@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 07/15] t1510 (worktree): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T04:39:08Z","receivedAt":"2011-12-08T04:39:08Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder wrote:\n> [...]\n> On some shells, like /usr/xpg4/bin/sh on Solaris, unset returns nonzero\n> status when the variable passed was already unset.  Will this work on\n> such platforms, or does it need to be changed to use sane_unset?\n\nInteresting.  Thanks for catching this.\n\n-- Ram\n"},{"id":"180555","messageId":"CALkWK0nDqbzkbsCqeqQ4ywvnR-t1KapxBZsSWJLu+1K+UDQGzg@mail.gmail.com","threadId":"29092","inReplyTo":"20111207214716.GA2911@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 04/15] t1007 (hash-object): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T04:42:18Z","receivedAt":"2011-12-08T04:42:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n> [...]\n>\n> IMHO if the patches are only being sent to me, Junio, and the\n> mailing list (and not cc-ed to different people), then there's no\n> reason to split them up when they have the same topic.\n\nRight.  I'll re-roll with similar patches squashed together -- it's\neasier to review and fixup in this form.\n\n>> --- a/t/t1007-hash-object.sh\n>> +++ b/t/t1007-hash-object.sh\n>> @@ -154,13 +154,13 @@ test_expect_success 'check that --no-filters option works with --stdin-paths' '\n>>  pop_repo\n>>\n>>  for args in \"-w --stdin\" \"--stdin -w\"; do\n>> -     push_repo\n>> +     push_repo &&\n>>\n>>       test_expect_success \"hash from stdin and write to database ($args)\" '\n>>               test $example_sha1 = $(git hash-object $args < example)\n>> -     '\n>> +     ' &&\n>\n> I don't see how this would have any effect.  Is it intended?\n\nOops, looks like I was in a bit of a rush.  Good catch.\n\n-- Ram\n"},{"id":"180556","messageId":"CALkWK0nDHa-Kc9_0kCw-LCoykudvZOR-9JNASyLj_=uvn-eJ9A@mail.gmail.com","threadId":"29092","inReplyTo":"20111207215509.GC2911@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 08/15] t3200 (branch): fix && chaining","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T04:47:24Z","receivedAt":"2011-12-08T04:47:24Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n>\n>> --- a/t/t3200-branch.sh\n>> +++ b/t/t3200-branch.sh\n>> @@ -22,7 +22,7 @@ test_expect_success \\\n>>\n>>  test_expect_success \\\n>>      'git branch --help should not have created a bogus branch' '\n>> -     git branch --help </dev/null >/dev/null 2>/dev/null;\n>> +     git branch --help </dev/null >/dev/null 2>/dev/null &&\n>>       test_path_is_missing .git/refs/heads/--help\n>\n> Won't this break when running tests for the first time, before the git\n> manpages are installed?\n\nUsed test_might_fail to guard it this time.  Thanks.\n\n-- Ram\n"},{"id":"180559","messageId":"vpqmxb37ahp.fsf@bauges.imag.fr","threadId":"29092","inReplyTo":"1323286611-4806-1-git-send-email-artagnon@gmail.com","subject":"Re: &&-chaining tester","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-12-08T08:06:58Z","receivedAt":"2011-12-08T08:06:58Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Hm, involves a huge amount of janitorial work.  Anyway, thanks for\n> pointing me in the right direction- here's a small start.\n\nPatches 1-14 look good to me (with Jonathan's remarks). I didn't review\npatch 15.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"180574","messageId":"CALkWK0mScWsQanBu=cq-qiC3Ls=bYnjA4eiY4mUFs+kr9_OHdQ@mail.gmail.com","threadId":"29092","inReplyTo":"20111207222126.GF2911@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 15/15] t3040 (subprojects-basic): modernize style","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-08T13:04:39Z","receivedAt":"2011-12-08T13:04:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> It would be easier to read if each preimage test assertion were next to\n> the corresponding postimage test assertion.  Does \"git diff --patience\"\n> do better?\n\nNo, unfortunately.\n\n> If cleaning up the style anyway, I would write this as\n> [...]\n> But leaving it alone like you did is probably better. ;-)\n\nExercising restraint.  Otherwise, I'm tempted to fix every little nit\nin every test I'm touching in the series :P\n\n>> -test_expect_success 'check if fsck ignores the subprojects' \\\n>> -    'git fsck --full'\n>> +test_expect_success 'check if fsck ignores the subprojects' '\n>> +     git fsck --full\n>> +'\n>\n> Does this test imply that one of the subprojects is broken somehow?\n\nHm, you're right- the test doesn't seem to make any sense.  I'll deal\nwith it in another patch (unrelated to this series).\n\nThanks.\n\n-- Ram\n"},{"id":"180605","messageId":"7vzkf3hqor.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":"CALkWK0nNtvrLHxQv17jfrFQ=BcwLfgh7eE9X-nDCCYY0nsOskg@mail.gmail.com","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-08T18:19:16Z","receivedAt":"2011-12-08T18:19:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n>>  * On \"revert: make commit subjects in insn sheet optional\"\n>>\n>>   After finding the verb and advancing the pointer \"bol\" at the beginning of\n>>   the object name, end_of_object_name variable points at the first SP or LF\n>>   using strcspn(bol, \" \\n\"), but I wonder why we are not grabbing a run of\n>>   hexdigits instead, i.e. strspn(bol, \"0123456789abcdef\"). Is this so that\n>>   we can allow something like \"pick rr/revert-cherry-pick~3\"?\n>\n> Yes, it is exactly for that reason :)\n\nThat is not explained in the commit message, which in itself is a problem\nthat needs to be addressed, but ...\n\n>>   I also wonder if this should be (sorry for pcre) \"(pick|revert)\\s+(\\S+)\\s\"\n>>   instead, i.e. allow users with fat fingers to use one or more SP or even HT\n>>   to separate the verb and the operand.\n>\n> Hm, I'm not too enthusiastic about this change, because we don't\n> advertise hand-editing the instruction sheet yet:...\n\n... is inconsistent with the above.\n\nIf you plan to later allow editing and if you know what kind of editing\nyou are going to allow, it would be better to prepare the parser to accept\ninput with a reasonable slack, especially because you are already touching\nit in this series anyway.\n\nOn the other hand, if you do not want to think about (or do not want us to\nget distracted by thinking about) allowing editing in this series, which\nis also a sensible stance to take, the parser should be made to accept\nonly what the mechanism in the series would produce and error out\notherwise, leaving the entire \"the user now can edit and here are the rules\"\npart for a separate series that comes after this series is perfected.\n\n>>  * On \"revert: allow mixed pick and revert instructions\"\n>>\n>>   Reporting what we did not understand from parse_insn_line() is a good\n>>   idea, but I think the line number should be reported at the beginning\n>>   of the same line.\n>\n> Makes sense.  Do you like this?\n\nNot particularly.\n\n> -\t\treturn error(_(\"Unrecognized action: %.*s\"), (int)len, bol);\n> +\t\treturn error(_(\"Line %d: Unrecognized action: %.*s\"),\n\nIt may be just me, but personally I prefer to see which file of what line\nhad the problem, i.e. \"%s:%d: Unrecognized action:%s\".\n\nBut if there is not much point in reporting the filename because it always\nis the same, \"Unrecognized action (line %d): %s\" would also be fine.\n"},{"id":"180611","messageId":"CABPQNSYQ=nt9LYzXpQgfwV00e9AxOV3LKj6VCCO8xkMAXb-Lfg@mail.gmail.com","threadId":"29092","inReplyTo":"20111206185218.GB9492@sigill.intra.peff.net","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-12-08T19:44:15Z","receivedAt":"2011-12-08T19:44:15Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Dec 6, 2011 at 7:52 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Dec 06, 2011 at 12:22:06PM +0100, Erik Faye-Lund wrote:\n>\n>> >> * jk/upload-archive-use-start-command (2011-11-21) 1 commit\n>> >>  - upload-archive: use start_command instead of fork\n>> >>\n>> >> What's the status of this one?\n>> >\n>> > I think what you have in pu is good, but of course I didn't actually\n>> > test it on Windows. Erik?\n>> >\n>>\n>> I tried to check out ee27ca4 and build, and got hit by a wall of warnings:\n>\n> I think you are on the wrong topic. ee27ca4 is the maint topic for\n> \"don't let remote clients get unreachable commits\", and is based on the\n> ancient 1.6.2. Which is why you are getting all of those warnings.\n>\n\nYou are indeed right. Thanks for spotting :)\n\n> The topic in question is jk/upload-archive-use-start-command, which is\n> at 1bc01ef, and should be based on recent-ish master.\n\nt5000-tar-tree pass for that one as well. No warnings this time :P\n"},{"id":"180617","messageId":"7vr50ehi1n.fsf@alter.siamese.dyndns.org","threadId":"29092","inReplyTo":"CABPQNSYQ=nt9LYzXpQgfwV00e9AxOV3LKj6VCCO8xkMAXb-Lfg@mail.gmail.com","subject":"Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-08T21:25:56Z","receivedAt":"2011-12-08T21:25:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n>> The topic in question is jk/upload-archive-use-start-command, which is\n>> at 1bc01ef, and should be based on recent-ish master.\n>\n> t5000-tar-tree pass for that one as well. No warnings this time :P\n\nThanks; let's move it forward, then.\n"}]}