What's cooking in git.git (Nov 2012, #06; Mon, 19)

What's cooking in git.git (Nov 2012, #06; Mon, 19)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

Bunch of topics have been merged to 'next'.

We are at the beginning of the 5th week of this release cycle
(cf. http://tinyurl.com/gitcal), and I've moved many topics to the
Stalled category, which will be discarded without prejudice soonish
unless there are some updates.  I am still a bit behind on some
topics and already posted rerolls may have to be pulled in.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

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

* nd/pathspec-wildcard (2012-11-19) 4 commits
 - tree_entry_interesting: do basedir compare on wildcard patterns when possible
 - pathspec: apply "*.c" optimization from exclude
 - pathspec: do exact comparison on the leading non-wildcard part
 - pathspec: save the non-wildcard length part

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

* pf/editor-ignore-sigint (2012-11-11) 5 commits
 - launch_editor: propagate SIGINT from editor to git
 - run-command: do not warn about child death by SIGINT
 - run-command: drop silent_exec_failure arg from wait_or_whine
 - launch_editor: ignore SIGINT while the editor has control
 - launch_editor: refactor to use start/finish_command

 Avoid confusing cases where the user hits Ctrl-C while in the editor
 session, not realizing git will receive the signal. Since most editors
 will take over the terminal and will block SIGINT, this is not likely
 to confuse anyone.

 Some people raised issues with emacsclient, which are addressed by this
 re-roll. It should probably also handle SIGQUIT, and there were a
 handful of other review comments.

 Expecting a re-roll.


* pp/gitweb-config-underscore (2012-11-08) 1 commit
 - gitweb: make remote_heads config setting work

 The key "gitweb.remote_heads" is not legal git config; this maps it to
 "gitweb.remoteheads".

 Junio raised a good point about the implementation for three-level
 variables.

 Expecting a re-roll.


* mo/cvs-server-updates (2012-10-16) 10 commits
 - cvsserver Documentation: new cvs ... -r support
 - cvsserver: add t9402 to test branch and tag refs
 - cvsserver: support -r and sticky tags for most operations
 - cvsserver: Add version awareness to argsfromdir
 - cvsserver: generalize getmeta() to recognize commit refs
 - cvsserver: implement req_Sticky and related utilities
 - cvsserver: add misc commit lookup, file meta data, and file listing functions
 - cvsserver: define a tag name character escape mechanism
 - cvsserver: cleanup extra slashes in filename arguments
 - cvsserver: factor out git-log parsing logic

 Needs review by folks interested in cvsserver.


* jc/apply-trailing-blank-removal (2012-10-12) 1 commit
 - apply.c:update_pre_post_images(): the preimage can be truncated

 Fix to update_pre_post_images() that did not take into account the
 possibility that whitespace fix could shrink the preimage and
 change the number of lines in it.

 Extra set of eyeballs appreciated.


* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
 - config: exit on error accessing any config file
 - doc: advertise GIT_CONFIG_NOSYSTEM
 - config: treat user and xdg config permission problems as errors
 - config, gitignore: failure to access with ENOTDIR is ok

 An RFC to deal with a situation where .config/git is a file and we
 notice .config/git/config is not readable due to ENOTDIR, not
 ENOENT; I think a bit more refactored approach to consistently
 address permission errors across config, exclude and attrs is
 desirable.  Don't we also need a check for an opposite situation
 where we open .config/git/config or .gitattributes for reading but
 they turn out to be directories?


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - dir.c: refactor is_path_excluded()
 - dir.c: refactor is_excluded()
 - dir.c: refactor is_excluded_from_list()
 - dir.c: rename excluded() to is_excluded()
 - dir.c: rename excluded_from_list() to is_excluded_from_list()
 - dir.c: rename path_excluded() to is_path_excluded()
 - dir.c: rename cryptic 'which' variable to more consistent name

 Duy helped to reroll this.

 Expecting a re-roll.


* aw/rebase-am-failure-detection (2012-10-11) 1 commit
 - rebase: Handle cases where format-patch fails

 I am unhappy a bit about the possible performance implications of
 having to store the output in a temporary file only for a rare case
 of format-patch aborting.


* jk/lua-hackery (2012-10-07) 6 commits
 - pretty: fix up one-off format_commit_message calls
 - Minimum compilation fixup
 - Makefile: make "lua" a bit more configurable
 - add a "lua" pretty format
 - add basic lua infrastructure
 - pretty: make some commit-parsing helpers more public

 Interesting exercise. When we do this for real, we probably would want
 to wrap a commit to make it more like an "object" with methods like
 "parents", etc.


* fc/remote-testgit-feature-done (2012-10-29) 1 commit
 - remote-testgit: properly check for errors

 Is this still in "Needs review" state?  Are poeple involved in the
 remote interface happy with this change?


* jk/send-email-sender-prompt (2012-11-15) 8 commits
 - send-email: do not prompt for explicit repo ident
 - Git.pm: teach "ident" to query explicitness
 - var: provide explicit/implicit ident information
 - var: accept multiple variables on the command line
 - ident: keep separate "explicit" flags for author and committer
 - ident: make user_ident_explicitly_given static
 - t7502: factor out autoident prerequisite
 - test-lib: allow negation of prerequisites

 Avoid annoying sender prompt in git-send-email, but only when it is
 safe to do so.

 Perhaps keep only the first three patches, and replace the rest
 with the one from Felipe that takes a much simpler approach (the
 rationale of that patch needs to be cleaned up first, along the
 lines Jeff outlined, though).  Frozen until that happens.


* nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
 - Unify appending signoff in format-patch, commit and sequencer

 I am not sure if the logic to refrain from adding a sign-off based
 on the existing run of sign-offs is done correctly in this change.


* nd/pretty-placeholder-with-color-option (2012-09-30) 9 commits
 . pretty: support %>> that steal trailing spaces
 . pretty: support truncating in %>, %< and %><
 . pretty: support padding placeholders, %< %> and %><
 . pretty: two phase conversion for non utf-8 commits
 . utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
 . utf8.c: move display_mode_esc_sequence_len() for use by other functions
 . pretty: support %C(auto[,N]) to turn on coloring on next placeholder(s)
 . pretty: split parsing %C into a separate function
 . pretty: share code between format_decoration and show_decorations

 This causes warnings with -Wuninitialized, so I've ejected it from pu
 for the time being.


* rc/maint-complete-git-p4 (2012-09-24) 1 commit
  (merged to 'next' on 2012-10-29 at af52cef)
 + Teach git-completion about git p4

 Comment from Pete will need to be addressed in a follow-up patch.


* as/test-tweaks (2012-09-20) 7 commits
 - tests: paint unexpectedly fixed known breakages in bold red
 - tests: test the test framework more thoroughly
 - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
 - tests: refactor mechanics of testing in a sub test-lib
 - tests: paint skipped tests in bold blue
 - tests: test number comes first in 'not ok $count - $message'
 - tests: paint known breakages in bold yellow

 Various minor tweaks to the test framework to paint its output
 lines in colors that match what they mean better.

 Has the "is this really blue?" issue Peff raised resolved???


* jc/maint-name-rev (2012-09-17) 7 commits
 - describe --contains: use "name-rev --algorithm=weight"
 - name-rev --algorithm=weight: tests and documentation
 - name-rev --algorithm=weight: cache the computed weight in notes
 - name-rev --algorithm=weight: trivial optimization
 - name-rev: --algorithm option
 - name_rev: clarify the logic to assign a new tip-name to a commit
 - name-rev: lose unnecessary typedef

 "git name-rev" names the given revision based on a ref that can be
 reached in the smallest number of steps from the rev, but that is
 not useful when the caller wants to know which tag is the oldest one
 that contains the rev.  This teaches a new mode to the command that
 uses the oldest ref among those which contain the rev.

 I am not sure if this is worth it; for one thing, even with the help
 from notes-cache, it seems to make the "describe --contains" even
 slower. Also the command will be unusably slow for a user who does
 not have a write access (hence unable to create or update the
 notes-cache).

 Stalled mostly due to lack of responses.


* jc/xprm-generation (2012-09-14) 1 commit
 - test-generation: compute generation numbers and clock skews

 A toy to analyze how bad the clock skews are in histories of real
 world projects.

 Stalled mostly due to lack of responses.


* jc/blame-no-follow (2012-09-21) 2 commits
 - blame: pay attention to --no-follow
 - diff: accept --no-follow option

 Teaches "--no-follow" option to "git blame" to disable its
 whole-file rename detection.

 Stalled mostly due to lack of responses.


* jc/doc-default-format (2012-10-07) 2 commits
 - [SQAUSH] allow "cd Doc* && make DEFAULT_DOC_TARGET=..."
 - Allow generating a non-default set of documentation

 Need to address the installation half if this is to be any useful.


* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
 - graph.c: infinite loop in git whatchanged --graph -m

 The --graph code fell into infinite loop when asked to do what the
 code did not expect ;-)

 Anybody who worked on "--graph" wants to comment?
 Stalled mostly due to lack of responses.


* jc/add-delete-default (2012-08-13) 1 commit
 - git add: notice removal of tracked paths by default

 "git add dir/" updated modified files and added new files, but does
 not notice removed files, which may be "Huh?" to some users.  They
 can of course use "git add -A dir/", but why should they?

 Resurrected from graveyard, as I thought it was a worthwhile thing
 to do in the longer term.

 Waiting for comments.


* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

 When the user does not specify what remote to interact with, we
 often attempt to use 'origin'.  This can now be customized via a
 configuration variable.

 Expecting a re-roll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* mh/ceiling (2012-10-29) 8 commits
 - string_list_longest_prefix(): remove function
 - setup_git_directory_gently_1(): resolve symlinks in ceiling paths
 - longest_ancestor_length(): require prefix list entries to be normalized
 - longest_ancestor_length(): take a string_list argument for prefixes
 - longest_ancestor_length(): use string_list_split()
 - Introduce new function real_path_if_valid()
 - real_path_internal(): add comment explaining use of cwd
 - Introduce new static function real_path_internal()

 Elements of GIT_CEILING_DIRECTORIES list may not match the real
 pathname we obtain from getcwd(), leading the GIT_DIR discovery
 logic to escape the ceilings the user thought to have specified.

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

* jl/submodule-rm (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at bf4525d)
 + docs: move submodule section

 Documentation correction for d21240f (Merge branch
 'jl/submodule-rm', 2012-10-29) that needs to be fast-tracked.

 Will merge to 'master' in the sixth batch.


* sg/complete-help-undup (2012-11-14) 1 commit
  (merged to 'next' on 2012-11-18 at eadd0f3)
 + completion: remove 'help' duplicate from porcelain commands

 Will merge to 'master' in the seventh batch.


* bc/do-not-recurse-in-die (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 79d62a8)
 + usage.c: detect recursion in die routines and bail out immediately

 Will merge to 'master' in the seventh batch.


* cn/config-missing-path (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at c08b73c)
 + config: don't segfault when given --path with a missing value

 Will merge to 'master' in the sixth batch.


* jk/checkout-out-of-unborn (2012-11-15) 1 commit
  (merged to 'next' on 2012-11-18 at 7d2aa24)
 + checkout: print a message when switching unborn branches

 Will merge to 'master' in the sixth batch.


* mk/complete-tcsh (2012-11-16) 1 commit
  (merged to 'next' on 2012-11-19 at 8309029)
 + tcsh-completion re-using git-completion.bash

 Will merge to 'master' in the seventh batch.


* mm/status-push-pull-advise (2012-11-16) 1 commit
 - status: add advice on how to push/pull to tracking branch

 Will merge to 'next'.


* jk/maint-gitweb-xss (2012-11-12) 1 commit
  (merged to 'next' on 2012-11-14 at 7a667bc)
 + gitweb: escape html in rss title

 Fixes an XSS vulnerability in gitweb.

 Will merge to 'master' in the sixth batch.


* mg/replace-resolve-delete (2012-11-13) 1 commit
  (merged to 'next' on 2012-11-14 at fa785ae)
 + replace: parse revision argument for -d

 Be more user friendly to people using "git replace -d".

 Will merge to 'master' in the sixth batch.


* ml/cygwin-mingw-headers (2012-11-18) 2 commits
  (merged to 'next' on 2012-11-19 at f9964da)
 + USE CGYWIN_V15_WIN32API as macro to select api for cygwin
  (merged to 'next' on 2012-11-15 at 22e11b3)
 + Update cygwin.c for new mingw-64 win32 api headers

 Make git work on newer cygwin.

 Will merge to 'master' in the sixth batch.


* ta/doc-cleanup (2012-10-25) 6 commits
  (merged to 'next' on 2012-11-13 at e11fafd)
 + Documentation: build html for all files in technical and howto
 + Documentation/howto: convert plain text files to asciidoc
 + Documentation/technical: convert plain text files to asciidoc
 + Change headline of technical/send-pack-pipeline.txt to not confuse its content with content from git-send-pack.txt
 + Shorten two over-long lines in git-bisect-lk2009.txt by abbreviating some sha1
 + Split over-long synopsis in git-fetch-pack.txt into several lines

 Will merge to 'master' in the sixth batch.


* lt/diff-stat-show-0-lines (2012-10-17) 1 commit
  (merged to 'next' on 2012-11-19 at 0037290)
 + Fix "git diff --stat" for interesting - but empty - file changes

 We failed to mention a file without any content change but whose
 permission bit was modified, or (worse yet) a new file without any
 content in the "git diff --stat" output.

 Will merge to 'master' in the seventh batch.


* fc/zsh-completion (2012-11-19) 2 commits
 - completion: start moving to the new zsh completion
 - completion: add new zsh completion

 Replaced by shedding large changes to other independent topics.
 Any comments from zsh users?


* so/prompt-command (2012-10-17) 4 commits
  (merged to 'next' on 2012-10-25 at 79565a1)
 + coloured git-prompt: paint detached HEAD marker in red
 + Fix up colored git-prompt
 + show color hints based on state of the git tree
 + Allow __git_ps1 to be used in PROMPT_COMMAND

 Updates __git_ps1 so that it can be used as $PROMPT_COMMAND,
 instead of being used for command substitution in $PS1, to embed
 color escape sequences in its output.

 Will cook in 'next'.


* nd/wildmatch (2012-10-15) 13 commits
  (merged to 'next' on 2012-10-25 at 510e8df)
 + Support "**" wildcard in .gitignore and .gitattributes
 + wildmatch: make /**/ match zero or more directories
 + wildmatch: adjust "**" behavior
 + wildmatch: fix case-insensitive matching
 + wildmatch: remove static variable force_lower_case
 + wildmatch: make wildmatch's return value compatible with fnmatch
 + t3070: disable unreliable fnmatch tests
 + Integrate wildmatch to git
 + wildmatch: follow Git's coding convention
 + wildmatch: remove unnecessary functions
 + Import wildmatch from rsync
 + ctype: support iscntrl, ispunct, isxdigit and isprint
 + ctype: make sane_ctype[] const array

 Allows pathname patterns in .gitignore and .gitattributes files
 with double-asterisks "foo/**/bar" to match any number of directory
 hierarchies.

 I suspect that this needs to be plugged to pathspec matching code;
 otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
 commits that touch Documentation/git.txt, which would be confusing
 to the users.

 Will cook in 'next'.


* jh/update-ref-d-through-symref (2012-10-21) 2 commits
  (merged to 'next' on 2012-11-19 at 6bcca4c)
 + Fix failure to delete a packed ref through a symref
 + t1400-update-ref: Add test verifying bug with symrefs in delete_ref()

 "update-ref -d --deref SYM" to delete a ref through a symbolic ref
 that points to it did not remove it correctly.

 Will merge to 'master' in the seventh batch.


* jk/config-ignore-duplicates (2012-10-29) 9 commits
  (merged to 'next' on 2012-10-29 at 67fa0a2)
 + builtin/config.c: Fix a sparse warning
  (merged to 'next' on 2012-10-25 at 233df08)
 + git-config: use git_config_with_options
 + git-config: do not complain about duplicate entries
 + git-config: collect values instead of immediately printing
 + git-config: fix regexp memory leaks on error conditions
 + git-config: remove memory leak of key regexp
 + t1300: test "git config --get-all" more thoroughly
 + t1300: remove redundant test
 + t1300: style updates

 Drop duplicate detection from git-config; this lets it
 better match the internal config callbacks, which clears up
 some corner cases with includes.

 Will merge to 'master' in the sixth batch.


* fc/completion-test-simplification (2012-11-16) 6 commits
 - completion: simplify __gitcomp() test helper
 - completion: refactor __gitcomp related tests
 - completion: consolidate test_completion*() tests
 - completion: simplify tests using test_completion_long()
 - completion: standardize final space marker in tests
 - completion: add comment for test_completion()

 Clean up completion tests.  Use of conslidated helper may make
 instrumenting one particular test during debugging of the test
 itself, but I think that issue should be addressed in some other
 way (e.g. making sure individual tests in 9902 can be skipped).


* jk/pickaxe-textconv (2012-10-28) 2 commits
 - pickaxe: use textconv for -S counting
 - pickaxe: hoist empty needle check

 Use textconv filters when searching with "log -S".

 It probably should lose "are the textconv on the two sides the
 same?" check.


* fc/remote-bzr (2012-11-08) 5 commits
  (merged to 'next' on 2012-11-18 at 86add07)
 + remote-bzr: update working tree
 + remote-bzr: add support for remote repositories
 + remote-bzr: add support for pushing
 + remote-bzr: add simple tests
 + Add new remote-bzr transport helper

 New remote helper for bzr.

 Will merge to 'master' in the seventh batch.


* fc/remote-hg (2012-11-12) 20 commits
  (merged to 'next' on 2012-11-18 at 4a4f2e4)
 + remote-hg: avoid bad refs
 + remote-hg: try the 'tip' if no checkout present
 + remote-hg: fix compatibility with older versions of hg
 + remote-hg: add missing config for basic tests
 + remote-hg: the author email can be null
 + remote-hg: add option to not track branches
 + remote-hg: add extra author test
 + remote-hg: add tests to compare with hg-git
 + remote-hg: add bidirectional tests
 + test-lib: avoid full path to store test results
 + remote-hg: add basic tests
 + remote-hg: fake bookmark when there's none
 + remote-hg: add compat for hg-git author fixes
 + remote-hg: add support for hg-git compat mode
 + remote-hg: match hg merge behavior
 + remote-hg: make sure the encoding is correct
 + remote-hg: add support to push URLs
 + remote-hg: add support for remote pushing
 + remote-hg: add support for pushing
 + Add new remote-hg transport helper

 New remote helper for hg.

 Will merge to 'master' in the seventh batch.


* jk/maint-http-half-auth-fetch (2012-10-31) 2 commits
  (merged to 'next' on 2012-11-09 at af69926)
 + remote-curl: retry failed requests for auth even with gzip
 + remote-curl: hoist gzip buffer size to top of post_rpc

 Fixes fetch from servers that ask for auth only during the actual
 packing phase. This is not really a recommended configuration, but it
 cleans up the code at the same time.

 Will merge to 'master' in the sixth batch.


* kb/preload-index-more (2012-11-02) 1 commit
  (merged to 'next' on 2012-11-09 at a750ebd)
 + update-index/diff-index: use core.preloadindex to improve performance

 Use preloadindex in more places, which has a nice speedup on systems
 with slow stat calls (and even on Linux).

 Will merge to 'master' in the sixth batch.


* cr/push-force-tag-update (2012-11-19) 5 commits
 - push: update remote tags only with force
 - push: flag updates that require force
 - push: keep track of "update" state separately
 - push: add advice for rejected tag reference
 - push: return reject reasons via a mask

 Require "-f" for push to update a tag, even if it is a fast-forward.


* fc/fast-export-fixes (2012-11-08) 14 commits
 - fast-export: don't handle uninteresting refs
 - fast-export: make sure updated refs get updated
 - fast-export: fix comparison in tests
 - fast-export: trivial cleanup
 - remote-testgit: make clear the 'done' feature
 - remote-testgit: report success after an import
 - remote-testgit: exercise more features
 - remote-testgit: cleanup tests
 - remote-testgit: remove irrelevant test
 - remote-testgit: get rid of non-local functionality
 - Add new simplified git-remote-testgit
 - Rename git-remote-testgit to git-remote-testpy
 - remote-testgit: fix direction of marks
 - fast-export: avoid importing blob marks

 Improvements to fix fast-export bugs, including how refs pointing to
 already-seen commits are handled. An earlier 4-commit version of this
 series looked good to me, but this much-expanded version has not seen
 any comments.

 Looks like it has been re-rolled, but I haven't checked it out yet.

 Needs review.


* mh/alt-odb-string-list-cleanup (2012-11-08) 2 commits
  (merged to 'next' on 2012-11-13 at 2bf41d9)
 + link_alt_odb_entries(): take (char *, len) rather than two pointers
 + link_alt_odb_entries(): use string_list_split_in_place()

 Cleanups in the alternates code. Fixes a potential bug and makes the
 code much cleaner.

 Will merge to 'master' in the sixth batch.


* pw/maint-p4-rcs-expansion-newline (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at e90cc7c)
 + git p4: RCS expansion should not span newlines

 I do not have p4 to play with, but looks obviously correct to me.

 Will merge to 'master' in the sixth batch.


* rh/maint-gitweb-highlight-ext (2012-11-08) 1 commit
  (merged to 'next' on 2012-11-13 at c57d856)
 + gitweb.perl: fix %highlight_ext mappings

 Fixes a clever misuse of perl's list interpretation.

 Will merge to 'master' in the sixth batch.


* rr/submodule-diff-config (2012-11-18) 4 commits
  (merged to 'next' on 2012-11-19 at 355319e)
 + submodule: display summary header in bold
 + diff: rename "set" variable
 + diff: introduce diff.submodule configuration variable
 + Documentation: move diff.wordRegex from config.txt to diff-config.txt

 Lets "git diff --submodule=log" become the default via configuration.

 Will merge to 'master' in the seventh batch.

Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)

[Felipe Contreras]

On Tue, Nov 20, 2012 at 12:55 AM, Junio C Hamano <gitster@pobox.com> wrote:

> * fc/remote-testgit-feature-done (2012-10-29) 1 commit
>  - remote-testgit: properly check for errors

Pinging Sverre again.

I now think that we need a better solution with waitpid() to check the
status of the process, so that both import and export get proper error
checks. I found that out the hard way with a buggy remote helper.

I still think this patch is good regardless, but not so big of a
priority. I'm not going to purse it more, if it gets in, good, if not,
a waitpid() patch would take care of it.

Cheers.

-- 
Felipe Contreras

Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)

[Paul Fox]

junio c hamano wrote:
 > [Stalled]
 > 
 > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 >  - launch_editor: propagate SIGINT from editor to git
 >  - run-command: do not warn about child death by SIGINT
 >  - run-command: drop silent_exec_failure arg from wait_or_whine
 >  - launch_editor: ignore SIGINT while the editor has control
 >  - launch_editor: refactor to use start/finish_command
 > 
 >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 >  session, not realizing git will receive the signal. Since most editors
 >  will take over the terminal and will block SIGINT, this is not likely
 >  to confuse anyone.
 > 
 >  Some people raised issues with emacsclient, which are addressed by this
 >  re-roll. It should probably also handle SIGQUIT, and there were a
 >  handful of other review comments.
 > 
 >  Expecting a re-roll.

i don't have strong feelings on any of the later review comments
(though i guess the check on finish_command()'s return code needed
attention), and wasn't the last submitter, but would certainly like to
see this move forward.

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 26.1 degrees)

Re: What's cooking in git.git (Nov 2012, #06; Mon, 19)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Here are the topics that have been cooking.  Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
>
> Bunch of topics have been merged to 'next'.
>
> We are at the beginning of the 5th week of this release cycle
> (cf. http://tinyurl.com/gitcal), and I've moved many topics to the
> Stalled category, which will be discarded without prejudice soonish
> unless there are some updates.  I am still a bit behind on some
> topics and already posted rerolls may have to be pulled in.

It feels a bit too busy/loud to issue two "What's cooking" in a row,
so here is an informal update.

 - The following have graduated to 'master'.

     cn/config-missing-path
     jk/checkout-out-of-unborn
     jk/maint-gitweb-xss
     jk/maint-http-half-auth-fetch
     jl/submodule-rm
     kb/preload-index-more
     mg/replace-resolve-delete
     mh/alt-odb-string-list-cleanup
     ml/cygwin-mingw-headers
     pw/maint-p4-rcs-expansion-newline
     rh/maint-gitweb-highlight-ext
     ta/doc-cleanup

 - Many topics have been merged to 'maint' in preparation for 1.8.0.1.

     mg/maint-pull-suggest-upstream-to
     mm/maint-doc-commit-edit
     as/maint-doc-fix-no-post-rewrite
     rs/lock-correct-ref-during-delete
     rf/maint-mailmap-off-by-one
     jk/maint-diff-grep-textconv
     js/format-2047
     sz/maint-curl-multi-timeout
     po/maint-refs-replace-docs
     ph/pull-rebase-detached
     mm/maint-doc-remote-tracking
     rs/branch-del-symref
     nd/grep-true-path
     jc/grep-pcre-loose-ends (early part)
     da/mergetools-p4
     jc/test-say-color-avoid-echo-escape
     bw/config-lift-variable-name-length-limit

 - A few topics have been resurrected from the stalled category to
   cooking:

     pp/gitweb-config-underscore
     jc/apply-trailing-blank-removal

Topics currently in the Stalled category

[Junio C Hamano]

Here is a list of stalled topics I am having trouble deciding what
to do (the default is to dismiss them around feature freeze).

* fc/fast-export-fixes (2012-11-08) 14 commits

 Renaming of remote-testgit feels to be a mistake.  It probably
 should keep its source in remote-testgit.bash and generate it, and
 moreover, if it wants to rename remote-testgit.py to remote-testpy,
 the new one should be called remote-testbash.  There was one reroll
 after what used to be queued, but nobody seemed to be interested in
 reviewing the series.

 This mostly happened while I was away, and judging from the
 discussion around this topic (and earlier iterations), I do not
 feel comfortable merging this series (or v5 reroll) as-is.

 Help?


* pf/editor-ignore-sigint (2012-11-11) 5 commits

 Avoid confusing cases where the user hits Ctrl-C while in the editor
 session, not realizing git will receive the signal. Since most editors
 will take over the terminal and will block SIGINT, this is not likely
 to confuse anyone.

 Some people raised issues with emacsclient, which are addressed by this
 re-roll. It should probably also handle SIGQUIT, and there were a
 handful of other review comments.

 Anybody interested in moving this forward?
 

* mo/cvs-server-updates (2012-10-16) 10 commits
 - cvsserver Documentation: new cvs ... -r support
 - cvsserver: add t9402 to test branch and tag refs
 - cvsserver: support -r and sticky tags for most operations
 - cvsserver: Add version awareness to argsfromdir
 - cvsserver: generalize getmeta() to recognize commit refs
 - cvsserver: implement req_Sticky and related utilities
 - cvsserver: add misc commit lookup, file meta data, and file listing functions
 - cvsserver: define a tag name character escape mechanism
 - cvsserver: cleanup extra slashes in filename arguments
 - cvsserver: factor out git-log parsing logic

 Needs review by folks interested in cvsserver.


* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
 - config: exit on error accessing any config file
 - doc: advertise GIT_CONFIG_NOSYSTEM
 - config: treat user and xdg config permission problems as errors
 - config, gitignore: failure to access with ENOTDIR is ok

 An RFC to deal with a situation where .config/git is a file and we
 notice .config/git/config is not readable due to ENOTDIR, not
 ENOENT; I think a bit more refactored approach to consistently
 address permission errors across config, exclude and attrs may be
 desirable.

 Should we merge this as-is and build on top?  What are the chances
 of potential regressions?


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - dir.c: refactor is_path_excluded()
 - dir.c: refactor is_excluded()
 - dir.c: refactor is_excluded_from_list()
 - dir.c: rename excluded() to is_excluded()
 - dir.c: rename excluded_from_list() to is_excluded_from_list()
 - dir.c: rename path_excluded() to is_path_excluded()
 - dir.c: rename cryptic 'which' variable to more consistent name

 Duy helped to reroll this, but it seems that there weren't any
 activity since then during my absense.


* fc/remote-testgit-feature-done (2012-10-29) 1 commit
 - remote-testgit: properly check for errors

 Is this still in "Needs review" state?  Are people involved in the
 remote interface happy with this change?


* jk/send-email-sender-prompt (2012-11-15) 8 commits
 - send-email: do not prompt for explicit repo ident
 - Git.pm: teach "ident" to query explicitness
 - var: provide explicit/implicit ident information
 - var: accept multiple variables on the command line
 - ident: keep separate "explicit" flags for author and committer
 - ident: make user_ident_explicitly_given static
 - t7502: factor out autoident prerequisite
 - test-lib: allow negation of prerequisites

 Avoid annoying sender prompt in git-send-email, but only when it is
 safe to do so.

 Perhaps keep only the first three patches, and replace the rest
 with the one from Felipe that takes a much simpler approach (the
 rationale of that patch needs to be cleaned up first, along the
 lines Jeff outlined, though).  Frozen until that happens.


* nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
 - Unify appending signoff in format-patch, commit and sequencer

 I am not sure if the logic to refrain from adding a sign-off based
 on the existing run of sign-offs is done correctly in this change.


* rc/maint-complete-git-p4 (2012-09-24) 1 commit
  (merged to 'next' on 2012-10-29 at af52cef)
 + Teach git-completion about git p4

 Comment from Pete will need to be addressed in a follow-up patch.


* as/test-tweaks (2012-09-20) 7 commits
 - tests: paint unexpectedly fixed known breakages in bold red
 - tests: test the test framework more thoroughly
 - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
 - tests: refactor mechanics of testing in a sub test-lib
 - tests: paint skipped tests in bold blue
 - tests: test number comes first in 'not ok $count - $message'
 - tests: paint known breakages in bold yellow

 Various minor tweaks to the test framework to paint its output
 lines in colors that match what they mean better.

 Has the "is this really blue?" issue Peff raised resolved???


* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
 - graph.c: infinite loop in git whatchanged --graph -m

 The --graph code fell into infinite loop when asked to do what the
 code did not expect ;-)

 Anybody who worked on "--graph" wants to comment?
 Stalled mostly due to lack of responses.


* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

 When the user does not specify what remote to interact with, we
 often attempt to use 'origin'.  This can now be customized via a
 configuration variable.

 Expecting a re-roll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* mh/ceiling (2012-10-29) 8 commits
 - string_list_longest_prefix(): remove function
 - setup_git_directory_gently_1(): resolve symlinks in ceiling paths
 - longest_ancestor_length(): require prefix list entries to be normalized
 - longest_ancestor_length(): take a string_list argument for prefixes
 - longest_ancestor_length(): use string_list_split()
 - Introduce new function real_path_if_valid()
 - real_path_internal(): add comment explaining use of cwd
 - Introduce new static function real_path_internal()

 Elements of GIT_CEILING_DIRECTORIES list may not match the real
 pathname we obtain from getcwd(), leading the GIT_DIR discovery
 logic to escape the ceilings the user thought to have specified.

 I think the fear that this would regress the intended use case of
 the environment variable turned out to be unfounded during the
 discussion.

 Should we merge this as-is to 'next', cook for a while to make sure
 nobody screams?

Thanks.

Re: Topics currently in the Stalled category

[Paul Fox]

In-reply-to: <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org> (sfid-20121120_190548_379327_D3EE7D14)
References: <7vpq39up0m.fsf@alter.siamese.dyndns.org> <7vy5hvq1ey.fsf@alter.siamese.dyndns.org> <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org> (sfid-20121120_190548_379327_D3EE7D14)
Fcc: outbox
--------

junio c hamano wrote:
 > Here is a list of stalled topics I am having trouble deciding what
 > to do (the default is to dismiss them around feature freeze).
...
 > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > 
 >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 >  session, not realizing git will receive the signal. Since most editors
 >  will take over the terminal and will block SIGINT, this is not likely
 >  to confuse anyone.
 > 
 >  Some people raised issues with emacsclient, which are addressed by this
 >  re-roll. It should probably also handle SIGQUIT, and there were a
 >  handful of other review comments.
 > 
 >  Anybody interested in moving this forward?

i started this, but then jeff took it and ran with it and made it
right.  i think the remaining changes are small -- if jeff would
prefer, i can probably finish it.  (but i won't guarantee not to
mess up the "From:" lines.  :-)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 36.0 degrees)

Re: Topics currently in the Stalled category

[Felipe Contreras]

On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Here is a list of stalled topics I am having trouble deciding what
> to do (the default is to dismiss them around feature freeze).
>
> * fc/fast-export-fixes (2012-11-08) 14 commits
>
>  Renaming of remote-testgit feels to be a mistake.  It probably
>  should keep its source in remote-testgit.bash and generate it,

Why generate it? There's nothing to generate. python's source code
needs regeneration, bash's code doesn't.

> and
>  moreover, if it wants to rename remote-testgit.py to remote-testpy,
>  the new one should be called remote-testbash.

No, remote-testbash is not testing anything that is specific to bash,
it's testing the remote helper framework itself. It could be written
in Ruby, or Python, or C, or anything.

remote-testgit.py is *not* testing the remote helper framework, it's
testing the Python-specific remote helper framework.

IOW. remote-testgit tests this:

  transport-helper.c

remote-testpy tests this:

  git_remote_helpers/Makefile
  git_remote_helpers/__init__.py
  git_remote_helpers/git/__init__.py
  git_remote_helpers/git/exporter.py
  git_remote_helpers/git/git.py
  git_remote_helpers/git/importer.py
  git_remote_helpers/git/non_local.py
  git_remote_helpers/git/repo.py
  git_remote_helpers/setup.cfg
  git_remote_helpers/setup.py
  git_remote_helpers/util.py

Cheers.

-- 
Felipe Contreras

Re: Topics currently in the Stalled category

[Krzysztof Mazur]

On Tue, Nov 20, 2012 at 09:46:47PM -0500, Paul Fox wrote:
> junio c hamano wrote:
>  > Here is a list of stalled topics I am having trouble deciding what
>  > to do (the default is to dismiss them around feature freeze).
> ...
>  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
>  > 
>  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
>  >  session, not realizing git will receive the signal. Since most editors
>  >  will take over the terminal and will block SIGINT, this is not likely
>  >  to confuse anyone.
>  > 
>  >  Some people raised issues with emacsclient, which are addressed by this
>  >  re-roll. It should probably also handle SIGQUIT, and there were a
>  >  handful of other review comments.
>  > 
>  >  Anybody interested in moving this forward?
> 
> i started this, but then jeff took it and ran with it and made it
> right.  i think the remaining changes are small -- if jeff would
> prefer, i can probably finish it.  (but i won't guarantee not to
> mess up the "From:" lines.  :-)
> 

I'm also interested. I sometimes use ":r !command" in vim, so far I never
needed to use Ctrl-C, but maybe in future.

The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

Krzysiek

Re: Topics currently in the Stalled category

[Marc Branchaud]

On 12-11-20 07:05 PM, Junio C Hamano wrote:
> Here is a list of stalled topics I am having trouble deciding what
> to do (the default is to dismiss them around feature freeze).
> 

[ snip ]

> 
> * mb/remote-default-nn-origin (2012-07-11) 6 commits
>  - Teach get_default_remote to respect remote.default.
>  - Test that plain "git fetch" uses remote.default when on a detached HEAD.
>  - Teach clone to set remote.default.
>  - Teach "git remote" about remote.default.
>  - Teach remote.c about the remote.default configuration setting.
>  - Rename remote.c's default_remote_name static variables.
> 
>  When the user does not specify what remote to interact with, we
>  often attempt to use 'origin'.  This can now be customized via a
>  configuration variable.
> 
>  Expecting a re-roll.
> 
>  "The first remote becomes the default" bit is better done as a
>  separate step.

This is still on my list of things to do soon.  Unfortunately "soon" seems to
be rather perpetual these days.

If you're tired of carrying the branch feel free to dismiss it and I'll
resurrect the topic when "soon" finally comes around.

		M.

Re: Topics currently in the Stalled category

[Jeff King]

On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:

> >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
> >  > 
> >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
> >  >  session, not realizing git will receive the signal. Since most editors
> >  >  will take over the terminal and will block SIGINT, this is not likely
> >  >  to confuse anyone.
> >  > 
> >  >  Some people raised issues with emacsclient, which are addressed by this
> >  >  re-roll. It should probably also handle SIGQUIT, and there were a
> >  >  handful of other review comments.
> >  > 
> >  >  Anybody interested in moving this forward?
> > 
> > i started this, but then jeff took it and ran with it and made it
> > right.  i think the remaining changes are small -- if jeff would
> > prefer, i can probably finish it.  (but i won't guarantee not to
> > mess up the "From:" lines.  :-)
> > 
> 
> I'm also interested. I sometimes use ":r !command" in vim, so far I never
> needed to use Ctrl-C, but maybe in future.
> 
> The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
there may have been one or two other minor comments in response to the
series). I am on vacation this week, but can revisit it next week. If
somebody wants to re-roll it in the meantime, that would be fine with
me.

-Peff

[1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
    nearly as widely used as SIGINT, but it sounds more like the
    principle of least surprise to treat them the same.

Re: Topics currently in the Stalled category

[Paul Fox]

jeff wrote:
 > On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:
 > 
 > > >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > > >  > 
 > > >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 > > >  >  session, not realizing git will receive the signal. Since most editors
 > > >  >  will take over the terminal and will block SIGINT, this is not likely
 > > >  >  to confuse anyone.
 > > >  > 
 > > >  >  Some people raised issues with emacsclient, which are addressed by this
 > > >  >  re-roll. It should probably also handle SIGQUIT, and there were a
 > > >  >  handful of other review comments.
 > > >  > 
 > > >  >  Anybody interested in moving this forward?
 > > > 
 > > > i started this, but then jeff took it and ran with it and made it
 > > > right.  i think the remaining changes are small -- if jeff would
 > > > prefer, i can probably finish it.  (but i won't guarantee not to
 > > > mess up the "From:" lines.  :-)
 > > > 
 > > 
 > > I'm also interested. I sometimes use ":r !command" in vim, so far I never
 > > needed to use Ctrl-C, but maybe in future.
 > > 
 > > The SIGINT part seems to be finished, we need to decide what about SIGQUIT.
 > 
 > My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
 > there may have been one or two other minor comments in response to the
 > series). I am on vacation this week, but can revisit it next week. If
 > somebody wants to re-roll it in the meantime, that would be fine with
 > me.
 > 
 > -Peff
 > 
 > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
 >     nearly as widely used as SIGINT, but it sounds more like the
 >     principle of least surprise to treat them the same.

i see no real point in treating them the same -- as you suggest, one
would only use SIGQUIT if SIGINT didn't work, and then you'd want them
to be treated differently.  so i'd be happy with the current code in
that regard.

i think krzysiek said that since editors usually catch SIGQUIT, git
should kill the editor when receiving SIGQUIT, essentially translating
the SIGQUIT to SIGTERM for the editor.  (please correct me if i
misunderstood.)  since well-behaved editors will die quickly anyway
(they get EIO on their next read from stdin), i'm not sure there's a
compelling reason for that extra step.

but i have no real objection to that behavior if others think it's
right -- there's certainly logic in saying that if git dies it should
ensure the editor does too.

(i'm away for the rest of the week also.)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 44.6 degrees)

Re: Topics currently in the Stalled category

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
>     nearly as widely used as SIGINT, but it sounds more like the
>     principle of least surprise to treat them the same.

Sounds sensible.  I wonder what happens when the editor is suspended
;-)

Re: Topics currently in the Stalled category

[Jeff King]

On Wed, Nov 21, 2012 at 11:53:04AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
> >     nearly as widely used as SIGINT, but it sounds more like the
> >     principle of least surprise to treat them the same.
> 
> Sounds sensible.  I wonder what happens when the editor is suspended
> ;-)

I think we would want to leave SIGTSTP alone; the editor should
typically respect it, and we would want to also pause until we get
SIGCONT (although even if we did continue, we would just be blocking on
wait() for the editor, anyway, so it is not a big deal).

Implicit in my "least surprise" comment above is that handling SIGQUIT
would match what system(3) does, so it makes sense to me to match that
(it also blocks SIGCHLD, but I do not think that really matters from a
user-visible perspective).

-Peff

Re: Topics currently in the Stalled category

[Nguyen Thai Ngoc Duy]

On Wed, Nov 21, 2012 at 7:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
>  - Unify appending signoff in format-patch, commit and sequencer
>
>  I am not sure if the logic to refrain from adding a sign-off based
>  on the existing run of sign-offs is done correctly in this change.

I'm not sure what's the right logic here. The original code in
format-patch has exactly one test just to verify that --signoff does
_something_. There's another signoff code in git-am.sh and we should
make sure at least the behavior is consistent. I guess I should start
over by writing new tests to record the current behavior, then fixes
and finally the unification. I think you can drop this for now.
-- 
Duy

[PATCH v2 0/4] nd/unify-appending-of-s-o-b

[Nguyễn Thái Ngọc Duy]

This round cherry-pick and commit only skip S-o-b if the last one is
the same while format-patch skips it if the same one appears
anywhere in S-o-b list. I don't have strong opinion about which way is
more correct. Though if we decide to change format-patch behavior, the
series may become a bit different.

Nguyễn Thái Ngọc Duy (4):
  t4014: more tests about appending s-o-b lines
  format-patch: stricter S-o-b detection
  format-patch: update append_signoff prototype
  Unify appending signoff in format-patch, commit and sequencer

 builtin/commit.c        |  10 ++-
 builtin/log.c           |  13 +---
 log-tree.c              |  66 +++++++++++++---
 log-tree.h              |   4 +
 revision.h              |   2 +-
 sequencer.c             |  65 +---------------
 sequencer.h             |   4 -
 t/t4014-format-patch.sh | 199 ++++++++++++++++++++++++++++++++++++++++++++++++
 t/t7501-commit.sh       |   2 +-
 9 files changed, 272 insertions(+), 93 deletions(-)

-- 
1.8.0.4.g5d0415a

[PATCH v2 1/4] t4014: more tests about appending s-o-b lines

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t4014-format-patch.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 959aa26..c8d5d29 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -891,4 +891,149 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '
 
+append_signoff()
+{
+	C=`git commit-tree HEAD^^{tree} -p HEAD` &&
+	git format-patch --stdout --signoff ${C}^..${C} |
+		tee append_signoff.patch |
+		sed -n "1,/^---$/p" |
+		grep -n -E "^Subject|Sign|^$"
+}
+
+test_expect_success 'signoff: commit with no body' '
+	append_signoff </dev/null >actual &&
+	cat <<\EOF | sed "s/EOL$//" >expected &&
+4:Subject: [PATCH] EOL
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject' '
+	echo subject | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject that does not end with NL' '
+	echo -n subject | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs and no trailing NL' '
+	printf "subject\n\nbody" | append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff-alike' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+Fooled-by-me: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end, no trailing NL' '
+	printf "subject\n\nSigned-off-by: C O Mitter <committer@example.com>" |
+		append_signoff >actual &&
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff NOT at the end' '
+	append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+Signed-off-by: my@house
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+12:Signed-off-by: my@house
+EOF
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.0.4.g5d0415a

[PATCH v2 2/4] format-patch: stricter S-o-b detection

[Nguyễn Thái Ngọc Duy]

S-o-b in the middle of a sentence, at the beginning of the sentence
but it is just because of text wrapping, or a full paragraph of valid
S-o-b in the middle of the message. All these are not S-o-b, but
detected as is. Fix them.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 log-tree.c              | 37 +++++++++++++++++++++++++++++++--
 t/t4014-format-patch.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index c894930..aff7c0a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -260,14 +260,47 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	int has_signoff = 0;
 	char *cp;
 
-	cp = sb->buf;
+	/*
+	 * Only search in the last paragrah, don't be fooled by a
+	 * paragraph full of valid S-o-b in the middle.
+	 */
+	cp = sb->buf + sb->len - 1;
+	while (cp > sb->buf) {
+		if (cp[0] == '\n' && cp[1] == '\n') {
+			cp += 2;
+			break;
+		}
+		cp--;
+	}
 
 	/* First see if we already have the sign-off by the signer */
 	while ((cp = strstr(cp, signed_off_by))) {
+		const char *s = cp;
+		cp += strlen(signed_off_by);
+
+		if (!has_signoff && s > sb->buf) {
+			/*
+			 * S-o-b in the middle of a sentence is not
+			 * really S-o-b
+			 */
+			if (s[-1] != '\n')
+				continue;
+
+			if (s - 1 > sb->buf && s[-2] == '\n')
+				; /* the first S-o-b */
+			else if (!detect_any_signoff(sb->buf, s - sb->buf))
+				/*
+				 * The previous line looks like an
+				 * S-o-b. There's still no guarantee
+				 * that it's an actual S-o-b. For that
+				 * we need to look back until we find
+				 * a blank line, which is too costly.
+				 */
+				continue;
+		}
 
 		has_signoff = 1;
 
-		cp += strlen(signed_off_by);
 		if (cp + signoff_len >= sb->buf + sb->len)
 			break;
 		if (strncmp(cp, signoff, signoff_len))
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index c8d5d29..d0b3c85 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -989,6 +989,60 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'signoff: not really a signoff' '
+	append_signoff <<\EOF >actual &&
+subject
+
+I want to mention about Signed-off-by: here.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:I want to mention about Signed-off-by: here.
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: not really a signoff (2)' '
+	append_signoff <<\EOF >actual &&
+subject
+
+My unfortunate
+Signed-off-by: example happens to be wrapped here.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:Signed-off-by: example happens to be wrapped here.
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'signoff: valid S-o-b paragraph in the middle' '
+	append_signoff <<\EOF >actual &&
+subject
+
+Signed-off-by: my@house
+Signed-off-by: your@house
+
+A lot of houses.
+EOF
+	cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: my@house
+10:Signed-off-by: your@house
+11:
+13:
+14:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'signoff: the same signoff at the end' '
 	append_signoff <<\EOF >actual &&
 subject
-- 
1.8.0.4.g5d0415a

[PATCH v2 3/4] format-patch: update append_signoff prototype

[Nguyễn Thái Ngọc Duy]

This is a preparation step for merging with append_signoff from
sequencer.c

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/log.c | 13 +------------
 log-tree.c    | 21 +++++++++++++--------
 revision.h    |  2 +-
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 09cf43e..f201070 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1053,7 +1053,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct commit *origin = NULL, *head = NULL;
 	const char *in_reply_to = NULL;
 	struct patch_ids ids;
-	char *add_signoff = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
 	int quiet = 0;
@@ -1148,16 +1147,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	if (do_signoff) {
-		const char *committer;
-		const char *endpos;
-		committer = git_committer_info(IDENT_STRICT);
-		endpos = strchr(committer, '>');
-		if (!endpos)
-			die(_("bogus committer info %s"), committer);
-		add_signoff = xmemdupz(committer, endpos - committer + 1);
-	}
-
 	for (i = 0; i < extra_hdr.nr; i++) {
 		strbuf_addstr(&buf, extra_hdr.items[i].string);
 		strbuf_addch(&buf, '\n');
@@ -1348,7 +1337,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		total++;
 		start_number--;
 	}
-	rev.add_signoff = add_signoff;
+	rev.add_signoff = do_signoff;
 	while (0 <= --nr) {
 		int shown;
 		commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index aff7c0a..7e50545 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -253,9 +253,11 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int flags)
 {
-	static const char signed_off_by[] = "Signed-off-by: ";
+	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					 getenv("GIT_COMMITTER_EMAIL")));
+	static const char sign_off_header[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
@@ -274,9 +276,9 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	}
 
 	/* First see if we already have the sign-off by the signer */
-	while ((cp = strstr(cp, signed_off_by))) {
+	while ((cp = strstr(cp, sign_off_header))) {
 		const char *s = cp;
-		cp += strlen(signed_off_by);
+		cp += strlen(sign_off_header);
 
 		if (!has_signoff && s > sb->buf) {
 			/*
@@ -317,9 +319,10 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
 	if (!has_signoff)
 		strbuf_addch(sb, '\n');
 
-	strbuf_addstr(sb, signed_off_by);
+	strbuf_addstr(sb, sign_off_header);
 	strbuf_add(sb, signoff, signoff_len);
 	strbuf_addch(sb, '\n');
+	free(signoff);
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -684,8 +687,10 @@ void show_log(struct rev_info *opt)
 	/*
 	 * And then the pretty-printed message itself
 	 */
-	if (ctx.need_8bit_cte >= 0)
-		ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+	if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+		ctx.need_8bit_cte =
+			has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					       getenv("GIT_COMMITTER_EMAIL")));
 	ctx.date_mode = opt->date_mode;
 	ctx.date_mode_explicit = opt->date_mode_explicit;
 	ctx.abbrev = opt->diffopt.abbrev;
@@ -696,7 +701,7 @@ void show_log(struct rev_info *opt)
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
-		append_signoff(&msgbuf, opt->add_signoff);
+		append_signoff(&msgbuf, 0);
 	if (opt->show_log_size) {
 		printf("log size %i\n", (int)msgbuf.len);
 		graph_show_oneline(opt->graph);
diff --git a/revision.h b/revision.h
index a95bd0b..af35325 100644
--- a/revision.h
+++ b/revision.h
@@ -136,7 +136,7 @@ struct rev_info {
 	int		numbered_files;
 	char		*message_id;
 	struct string_list *ref_message_ids;
-	const char	*add_signoff;
+	int              add_signoff;
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
-- 
1.8.0.4.g5d0415a

[PATCH v2 4/4] Unify appending signoff in format-patch, commit and sequencer

[Nguyễn Thái Ngọc Duy]

There are two implementations of append_signoff in log-tree.c and
sequencer.c, which do more or less the same thing. This patch removes
the sequencer.c's in favor of the format-patch's.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c  | 10 ++++++++-
 log-tree.c        | 14 ++++++++----
 log-tree.h        |  4 ++++
 sequencer.c       | 65 ++-----------------------------------------------------
 sequencer.h       |  4 ----
 t/t7501-commit.sh |  2 +-
 6 files changed, 26 insertions(+), 73 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index a17a5df..6d323d9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -698,7 +698,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 			previous = eol;
 		}
 
-		append_signoff(&sb, ignore_footer);
+		if (ignore_footer) {
+			struct strbuf footer = STRBUF_INIT;
+			strbuf_addstr(&footer, sb.buf + sb.len - ignore_footer);
+			strbuf_setlen(&sb, sb.len - ignore_footer);
+			append_signoff(&sb, SOB_IGNORE_SAME);
+			strbuf_addstr(&sb, footer.buf);
+			strbuf_release(&footer);
+		} else
+			append_signoff(&sb, SOB_IGNORE_SAME);
 	}
 
 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
diff --git a/log-tree.c b/log-tree.c
index 7e50545..e8d31d9 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -12,6 +12,8 @@
 
 struct decoration name_decoration = { "object names" };
 
+const char sign_off_header[] = "Signed-off-by: ";
+
 enum decoration_type {
 	DECORATION_NONE = 0,
 	DECORATION_REF_LOCAL,
@@ -207,7 +209,7 @@ void show_decorations(struct rev_info *opt, struct commit *commit)
 }
 
 /*
- * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches
+ * Search for "^[-A-Za-z0-9]+: [^@]+@" pattern. It usually matches
  * Signed-off-by: and Acked-by: lines.
  */
 static int detect_any_signoff(char *letter, int size)
@@ -243,6 +245,7 @@ static int detect_any_signoff(char *letter, int size)
 		}
 		if (('A' <= ch && ch <= 'Z') ||
 		    ('a' <= ch && ch <= 'z') ||
+		    ('0' <= ch && ch <= '9') ||
 		    ch == '-') {
 			seen_head = 1;
 			continue;
@@ -253,11 +256,10 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static void append_signoff(struct strbuf *sb, int flags)
+void append_signoff(struct strbuf *sb, int flags)
 {
 	char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
 					 getenv("GIT_COMMITTER_EMAIL")));
-	static const char sign_off_header[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
@@ -310,7 +312,11 @@ static void append_signoff(struct strbuf *sb, int flags)
 		if (!isspace(cp[signoff_len]))
 			continue;
 		/* we already have him */
-		return;
+		if (flags & SOB_IGNORE_SAME) {
+			if (cp[signoff_len + 1] == '\0')
+				return;
+		} else
+			return;
 	}
 
 	if (!has_signoff)
diff --git a/log-tree.h b/log-tree.h
index f5ac238..739f729 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -24,4 +24,8 @@ void load_ref_decorations(int flags);
 void get_patch_filename(struct commit *commit, const char *subject, int nr,
 			const char *suffix, struct strbuf *buf);
 
+#define SOB_IGNORE_SAME   1
+extern const char sign_off_header[];
+void append_signoff(struct strbuf *msgbuf, int flags);
+
 #endif
diff --git a/sequencer.c b/sequencer.c
index e3723d2..bc02a66 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -14,11 +14,10 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "log-tree.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
-const char sign_off_header[] = "Signed-off-by: ";
-
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -236,7 +235,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
 	rollback_lock_file(&index_lock);
 
 	if (opts->signoff)
-		append_signoff(msgbuf, 0);
+		append_signoff(msgbuf, SOB_IGNORE_SAME);
 
 	if (!clean) {
 		int i;
@@ -1016,63 +1015,3 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	save_opts(opts);
 	return pick_commits(todo_list, opts);
 }
-
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
-{
-	int ch;
-	int hit = 0;
-	int i, j, k;
-	int len = sb->len - ignore_footer;
-	int first = 1;
-	const char *buf = sb->buf;
-
-	for (i = len - 1; i > 0; i--) {
-		if (hit && buf[i] == '\n')
-			break;
-		hit = (buf[i] == '\n');
-	}
-
-	while (i < len - 1 && buf[i] == '\n')
-		i++;
-
-	for (; i < len; i = k) {
-		for (k = i; k < len && buf[k] != '\n'; k++)
-			; /* do nothing */
-		k++;
-
-		if ((buf[k] == ' ' || buf[k] == '\t') && !first)
-			continue;
-
-		first = 0;
-
-		for (j = 0; i + j < len; j++) {
-			ch = buf[i + j];
-			if (ch == ':')
-				break;
-			if (isalnum(ch) ||
-			    (ch == '-'))
-				continue;
-			return 0;
-		}
-	}
-	return 1;
-}
-
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
-{
-	struct strbuf sob = STRBUF_INIT;
-	int i;
-
-	strbuf_addstr(&sob, sign_off_header);
-	strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"),
-				getenv("GIT_COMMITTER_EMAIL")));
-	strbuf_addch(&sob, '\n');
-	for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
-		; /* do nothing */
-	if (prefixcmp(msgbuf->buf + i, sob.buf)) {
-		if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
-			strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
-		strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
-	}
-	strbuf_release(&sob);
-}
diff --git a/sequencer.h b/sequencer.h
index 9d57d57..99eb7fa 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -46,8 +46,4 @@ struct replay_opts {
 
 int sequencer_pick_revisions(struct replay_opts *opts);
 
-extern const char sign_off_header[];
-
-void append_signoff(struct strbuf *msgbuf, int ignore_footer);
-
 #endif
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 195e747..6dd4580 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -353,7 +353,7 @@ test_expect_success 'signoff gap' '
 
 	echo 3 >positive &&
 	git add positive &&
-	alt="Alt-RFC-822-Header: Value" &&
+	alt="Alt-RFC-822-Header: Va@lue" &&
 	git commit -s -m "welcome
 
 $alt" &&
-- 
1.8.0.4.g5d0415a

Re: [PATCH v2 0/4] nd/unify-appending-of-s-o-b

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This round cherry-pick and commit only skip S-o-b if the last one is
> the same...

That sounds sensible.  When the user asks us to add sign-off, we
should honor it, unless it is known that adding one is absolutely
unnecessary, which is "it already exists as the last one".  Ignoring
all other cases will lose information.

Among the non-merge commits in the kernel history, you see about 500
such commits where the same contributor signs off the patch more
than once, recording the flow of the patch; 106a4ee258 seems to be
the most recent example.

Thanks.

Re: Topics currently in the Stalled category

[Jeff King]

On Wed, Nov 21, 2012 at 04:31:11AM +0100, Felipe Contreras wrote:

> On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Here is a list of stalled topics I am having trouble deciding what
> > to do (the default is to dismiss them around feature freeze).
> >
> > * fc/fast-export-fixes (2012-11-08) 14 commits
> >
> >  Renaming of remote-testgit feels to be a mistake.  It probably
> >  should keep its source in remote-testgit.bash and generate it,
> 
> Why generate it? There's nothing to generate. python's source code
> needs regeneration, bash's code doesn't.

We fix up the #!-lines on all of the existing shell scripts (as well as
python and perl). Wouldn't we want to do the same for people who have
bash in an alternate location?

As the series is now, people with bash in their PATH, but not in
/bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
git-remote-testgit hardcodes /bin/bash).

-Peff

Re: Topics currently in the Stalled category

[Felipe Contreras]

On Wed, Nov 28, 2012 at 3:59 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 21, 2012 at 04:31:11AM +0100, Felipe Contreras wrote:
>
>> On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Here is a list of stalled topics I am having trouble deciding what
>> > to do (the default is to dismiss them around feature freeze).
>> >
>> > * fc/fast-export-fixes (2012-11-08) 14 commits
>> >
>> >  Renaming of remote-testgit feels to be a mistake.  It probably
>> >  should keep its source in remote-testgit.bash and generate it,
>>
>> Why generate it? There's nothing to generate. python's source code
>> needs regeneration, bash's code doesn't.
>
> We fix up the #!-lines on all of the existing shell scripts (as well as
> python and perl). Wouldn't we want to do the same for people who have
> bash in an alternate location?

'#!/usr/bin/env bash' should take care of people who have bash in an
alternate location, no?

-- 
Felipe Contreras

Re: Topics currently in the Stalled category

[Jeff King]

On Tue, Nov 27, 2012 at 09:59:28PM -0500, Jeff King wrote:

> > >  Renaming of remote-testgit feels to be a mistake.  It probably
> > >  should keep its source in remote-testgit.bash and generate it,
> > 
> > Why generate it? There's nothing to generate. python's source code
> > needs regeneration, bash's code doesn't.
> 
> We fix up the #!-lines on all of the existing shell scripts (as well as
> python and perl). Wouldn't we want to do the same for people who have
> bash in an alternate location?
> 
> As the series is now, people with bash in their PATH, but not in
> /bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
> git-remote-testgit hardcodes /bin/bash).

We could improve the test in t5801, but it is nice to let people on such
systems test it, as well. And the infrastructure might be useful if we
ever acquire more bash scripts.

There's a fair bit of boilerplate, but I think this squashable patch
would do it:

diff --git a/.gitignore b/.gitignore
index 46595db..3b636bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -124,6 +124,7 @@
 /git-remote-ftps
 /git-remote-fd
 /git-remote-ext
+/git-remote-testgit
 /git-remote-testsvn
 /git-remote-testpy
 /git-repack
diff --git a/Makefile b/Makefile
index 68f1ac2..19f6fd2 100644
--- a/Makefile
+++ b/Makefile
@@ -424,6 +424,7 @@ PROGRAM_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
+SCRIPT_BASH =
 SCRIPT_SH =
 SCRIPT_LIB =
 TEST_PROGRAMS_NEED_X =
@@ -473,9 +474,12 @@ SCRIPT_PERL += git-svn.perl
 SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
+SCRIPT_BASH += git-remote-testgit.bash
+
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
+	  $(patsubst %.bash,%,$(SCRIPT_BASH)) \
 	  git-instaweb
 
 ETAGS_TARGET = TAGS
@@ -571,9 +575,13 @@ endif
 ifndef PYTHON_PATH
 	PYTHON_PATH = /usr/bin/python
 endif
+ifndef BASH_PATH
+	BASH_PATH = /bin/bash
+endif
 
 export PERL_PATH
 export PYTHON_PATH
+export BASH_PATH
 
 LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a
@@ -1931,6 +1939,10 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON=NoThanks
 endif
 
+ifeq ($(BASH_PATH),)
+NO_BASH=NoThanks
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -2006,6 +2018,7 @@ gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
+BASH_PATH_SQ = $(subst ','\'',$(BASH_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 
@@ -2267,6 +2280,15 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
 	mv $@+ $@
 endif # NO_PYTHON
 
+ifndef NO_BASH
+$(patsubst %.bash,%,$(SCRIPT_BASH)):
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/bash|#!$(BASH_PATH_SQ)|' \
+	    $@.bash >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+endif
+
 configure: configure.ac GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $<+ && \
 	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -2599,6 +2621,7 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
+	@echo NO_BASH=\''$(subst ','\'',$(subst ','\'',$(NO_BASH)))'\' >>$@
 	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
 ifdef GIT_TEST_OPTS
 	@echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' >>$@
diff --git a/git-remote-testgit b/git-remote-testgit.bash
similarity index 100%
rename from git-remote-testgit
rename to git-remote-testgit.bash
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 456303b..d6ebc5e 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -7,7 +7,7 @@ test_description='Test remote-helper import and export commands'
 
 . ./test-lib.sh
 
-if ! type "${BASH-bash}" >/dev/null 2>&1; then
+if ! test_have_prereq BASH; then
 	skip_all='skipping remote-testgit tests, bash not available'
 	test_done
 fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..5300fa9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -675,6 +675,7 @@ esac
 ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
+test -z "$NO_BASH" && test_set_prereq BASH
 test -n "$USE_LIBPCRE" && test_set_prereq LIBPCRE
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
 

Re: Topics currently in the Stalled category

[Jeff King]

On Wed, Nov 28, 2012 at 04:10:07AM +0100, Felipe Contreras wrote:

> >> Why generate it? There's nothing to generate. python's source code
> >> needs regeneration, bash's code doesn't.
> >
> > We fix up the #!-lines on all of the existing shell scripts (as well as
> > python and perl). Wouldn't we want to do the same for people who have
> > bash in an alternate location?
> 
> '#!/usr/bin/env bash' should take care of people who have bash in an
> alternate location, no?

If that is where there env is. I do not recall offhand if that is a
problem for anyone. Still, I don't think it is that big a deal to treat
it like we do other scripts, and provide the usual boilerplate (which
also gets us NO_BASH if you have an old or broken bash and want to skip
the tests). I just sent a patch.

-Peff

Re: Topics currently in the Stalled category

[Felipe Contreras]

On Wed, Nov 28, 2012 at 4:11 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 27, 2012 at 09:59:28PM -0500, Jeff King wrote:
>
>> > >  Renaming of remote-testgit feels to be a mistake.  It probably
>> > >  should keep its source in remote-testgit.bash and generate it,
>> >
>> > Why generate it? There's nothing to generate. python's source code
>> > needs regeneration, bash's code doesn't.
>>
>> We fix up the #!-lines on all of the existing shell scripts (as well as
>> python and perl). Wouldn't we want to do the same for people who have
>> bash in an alternate location?
>>
>> As the series is now, people with bash in their PATH, but not in
>> /bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
>> git-remote-testgit hardcodes /bin/bash).
>
> We could improve the test in t5801, but it is nice to let people on such
> systems test it, as well. And the infrastructure might be useful if we
> ever acquire more bash scripts.
>
> There's a fair bit of boilerplate, but I think this squashable patch
> would do it:

Yeah, but I wonder what's the point of installing this script, it's
mostly for testing and reference, and to add a whole category for that
seems like overkill.

Cheers.

-- 
Felipe Contreras

Re: Topics currently in the Stalled category

[Jeff King]

On Wed, Nov 28, 2012 at 04:15:12AM +0100, Felipe Contreras wrote:

> > We could improve the test in t5801, but it is nice to let people on such
> > systems test it, as well. And the infrastructure might be useful if we
> > ever acquire more bash scripts.
> >
> > There's a fair bit of boilerplate, but I think this squashable patch
> > would do it:
> 
> Yeah, but I wonder what's the point of installing this script, it's
> mostly for testing and reference, and to add a whole category for that
> seems like overkill.

There's no point in installing it; I just didn't make the effort to
avoid doing so (note that testpy and testsvn are also installed, which
are in the same boat; it might make sense to split them all out like we
do for $TEST_PROGRAMS).

I agree it's an annoying amount of boilerplate, but it seems simpler
cognitively to me for it to behave as the other SCRIPT_* builds than to
do something simple but inconsistent.

I do not care enough to argue about it. We need to do something to fix
the impending test breakage on systems like Solaris. I have posted the
patch to handle BASH_PATH, so do what you want.

-Peff

Re: Topics currently in the Stalled category

[Felipe Contreras]

On Wed, Nov 28, 2012 at 4:22 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 28, 2012 at 04:15:12AM +0100, Felipe Contreras wrote:
>
>> > We could improve the test in t5801, but it is nice to let people on such
>> > systems test it, as well. And the infrastructure might be useful if we
>> > ever acquire more bash scripts.
>> >
>> > There's a fair bit of boilerplate, but I think this squashable patch
>> > would do it:
>>
>> Yeah, but I wonder what's the point of installing this script, it's
>> mostly for testing and reference, and to add a whole category for that
>> seems like overkill.
>
> There's no point in installing it; I just didn't make the effort to
> avoid doing so (note that testpy and testsvn are also installed, which
> are in the same boat; it might make sense to split them all out like we
> do for $TEST_PROGRAMS).
>
> I agree it's an annoying amount of boilerplate, but it seems simpler
> cognitively to me for it to behave as the other SCRIPT_* builds than to
> do something simple but inconsistent.
>
> I do not care enough to argue about it. We need to do something to fix
> the impending test breakage on systems like Solaris. I have posted the
> patch to handle BASH_PATH, so do what you want.

I'm not objecting to the change, I'm simply wondering.

Personally I think switching to '/usr/bin/env bash' should be enough
for now. Doing a grep on my git installation throws this:

% grep '/usr/bin/env' -r /opt/git
/opt/git/lib/python2.7/site-packages/git_remote_helpers/git/git.py:#!/usr/bin/env
python
/opt/git/lib/python2.7/site-packages/git_remote_helpers/__init__.py:#!/usr/bin/env
python
/opt/git/lib/python2.7/site-packages/git_remote_helpers/util.py:#!/usr/bin/env
python
/opt/git/libexec/git-core/git-instaweb:#!/usr/bin/env ruby

So it's doubtful a lack of /usr/bin/env would cause any more breakages
than it already does for test-git.

And this just landed on 'pu', I don't think there's anything big
impending. If the /usr/bin/env solution turned out to be not enough
for some reason, we can deal with it then. That's my opinion.

Cheers.

-- 
Felipe Contreras

Re: Topics currently in the Stalled category

[Adam Spiers]

On Wed, Nov 21, 2012 at 12:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Here is a list of stalled topics I am having trouble deciding what
> to do (the default is to dismiss them around feature freeze).

[snipped]

> * as/check-ignore (2012-11-08) 14 commits
>  - t0007: fix tests on Windows
>  - Documentation/check-ignore: we show the deciding match, not the first
>  - Add git-check-ignore sub-command
>  - dir.c: provide free_directory() for reclaiming dir_struct memory
>  - pathspec.c: move reusable code from builtin/add.c
>  - dir.c: refactor treat_gitlinks()
>  - dir.c: keep track of where patterns came from
>  - dir.c: refactor is_path_excluded()
>  - dir.c: refactor is_excluded()
>  - dir.c: refactor is_excluded_from_list()
>  - dir.c: rename excluded() to is_excluded()
>  - dir.c: rename excluded_from_list() to is_excluded_from_list()
>  - dir.c: rename path_excluded() to is_path_excluded()
>  - dir.c: rename cryptic 'which' variable to more consistent name
>
>  Duy helped to reroll this, but it seems that there weren't any
>  activity since then during my absense.

I have been delayed several times, but I finally resumed work on
another re-roll.  I don't think there is any major reworking required;
just a number of small tweaks.

> * as/test-tweaks (2012-09-20) 7 commits
>  - tests: paint unexpectedly fixed known breakages in bold red
>  - tests: test the test framework more thoroughly
>  - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
>  - tests: refactor mechanics of testing in a sub test-lib
>  - tests: paint skipped tests in bold blue
>  - tests: test number comes first in 'not ok $count - $message'
>  - tests: paint known breakages in bold yellow
>
>  Various minor tweaks to the test framework to paint its output
>  lines in colors that match what they mean better.
>
>  Has the "is this really blue?" issue Peff raised resolved???

I have a re-roll of this ready - just need to rebase to latest master,
do a final sanity check, and then send.

Sorry again for the delays.

Re: [PATCH v2 1/4] t4014: more tests about appending s-o-b lines

[Torsten Bögershausen]

On 22.11.12 17:38, Nguyễn Thái Ngọc Duy wrote:
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  t/t4014-format-patch.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 145 insertions(+)
> +	echo -n subject | append_signoff >actual &&


"echo -n" is not portable, and we use printf everywhere. 
I found one "echo -n" in line  996.

Can we squeeze that in, before going to next?

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 6cfad13..f460930 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -993,7 +993,7 @@ EOF
 '
 
 test_expect_success 'signoff: commit with only subject that does not end with NL' '
-       echo -n subject | append_signoff >actual &&
+       printf subject | append_signoff >actual &&
        cat >expected <<\EOF &&
 4:Subject: [PATCH] subject
 8:

Re: [PATCH v2 1/4] t4014: more tests about appending s-o-b lines

[Brandon Casey]

On Sat, Dec 1, 2012 at 11:06 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 22.11.12 17:38, Nguyễn Thái Ngọc Duy wrote:
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  t/t4014-format-patch.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 145 insertions(+)
>> +     echo -n subject | append_signoff >actual &&
>
>
> "echo -n" is not portable, and we use printf everywhere.
> I found one "echo -n" in line  996.
>
> Can we squeeze that in, before going to next?

I got it.  I'll squash it into the next series.

-Brandon