* What's cooking in git.git (Jan 2013, #09; Fri, 25)
From: Junio C Hamano @ 2013-01-26 0:23 UTC (permalink / raw)
To: git
What's cooking in git.git (Jan 2013, #09; Fri, 25)
--------------------------------------------------
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
As usual, this cycle is expected to last for 8 to 10 weeks, with a
preview -rc0 sometime in the middle of next month.
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
--------------------------------------------------
[Graduated to "master"]
* as/check-ignore (2013-01-16) 13 commits
(merged to 'next' on 2013-01-18 at ef45aff)
+ clean.c, ls-files.c: respect encapsulation of exclude_list_groups
(merged to 'next' on 2013-01-14 at 9df2afc)
+ t0008: avoid brace expansion
+ add git-check-ignore sub-command
+ setup.c: document get_pathspec()
+ add.c: extract new die_if_path_beyond_symlink() for reuse
+ add.c: extract check_path_for_gitlink() from treat_gitlinks() for reuse
+ pathspec.c: rename newly public functions for clarity
+ add.c: move pathspec matchers into new pathspec.c for reuse
+ add.c: remove unused argument from validate_pathspec()
+ dir.c: improve docs for match_pathspec() and match_pathspec_depth()
+ dir.c: provide clear_directory() for reclaiming dir_struct memory
+ dir.c: keep track of where patterns came from
+ dir.c: use a single struct exclude_list per source of excludes
Add a new command "git check-ignore" for debugging .gitignore
files. The variable names may want to get cleaned up but that can
be done in-tree.
* as/pre-push-hook (2013-01-18) 3 commits
(merged to 'next' on 2013-01-18 at 37fc4e8)
+ Add sample pre-push hook script
+ push: Add support for pre-push hooks
+ hooks: Add function to check if a hook exists
Add an extra hook so that "git push" that is run without making
sure what is being pushed is sane can be checked and rejected (as
opposed to the user deciding not pushing).
* ch/add-auto-submitted-in-sample-post-receive-email (2013-01-17) 1 commit
(merged to 'next' on 2013-01-18 at e3205db)
+ Add Auto-Submitted header to post-receive-email
Mark e-mails coming from automated processes should be marked as
such; update a sample hook to do so.
* cr/push-force-tag-update (2013-01-16) 1 commit
(merged to 'next' on 2013-01-18 at c9091d5)
+ push: fix "refs/tags/ hierarchy cannot be updated without --force"
(this branch is used by jc/push-reject-reasons.)
Regression fix, not to say "already exists" when we traditionally
said "non fast-forward'.
* jc/doc-maintainer (2013-01-03) 2 commits
(merged to 'next' on 2013-01-11 at f35d582)
+ howto/maintain: mark titles for asciidoc
+ Documentation: update "howto maintain git"
Describe tools for automation that were invented since this
document was originally written.
* jk/suppress-clang-warning (2013-01-16) 1 commit
(merged to 'next' on 2013-01-18 at 7c0bda7)
+ fix clang -Wunused-value warnings for error functions
* mh/imap-send-shrinkage (2013-01-15) 14 commits
(merged to 'next' on 2013-01-18 at 1b7c5ba)
+ imap-send.c: simplify logic in lf_to_crlf()
+ imap-send.c: fold struct store into struct imap_store
+ imap-send.c: remove unused field imap_store::uidvalidity
+ imap-send.c: use struct imap_store instead of struct store
+ imap-send.c: remove unused field imap_store::trashnc
+ imap-send.c: remove namespace fields from struct imap
+ imap-send.c: remove struct imap argument to parse_imap_list_l()
+ imap-send.c: inline parse_imap_list() in parse_list()
+ imap-send.c: remove some unused fields from struct store
+ imap-send.c: remove struct message
+ imap-send.c: remove struct store_conf
+ iamp-send.c: remove unused struct imap_store_conf
+ imap-send.c: remove struct msg_data
+ imap-send.c: remove msg_data::flags, which was always zero
Remove a lot of unused code from "git imap-send".
* mo/cvs-server-updates (2012-12-09) 18 commits
(merged to 'next' on 2013-01-08 at 75e2d11)
+ t9402: Use TABs for indentation
+ t9402: Rename check.cvsCount and check.list
+ t9402: Simplify git ls-tree
+ t9402: Add missing &&; Code style
+ t9402: No space after IO-redirection
+ t9402: Dont use test_must_fail cvs
+ t9402: improve check_end_tree() and check_end_full_tree()
+ t9402: sed -i is not portable
+ 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
Various git-cvsserver updates.
* nd/retire-fnmatch (2013-01-01) 7 commits
(merged to 'next' on 2013-01-07 at ab31f9b)
+ Makefile: add USE_WILDMATCH to use wildmatch as fnmatch
+ wildmatch: advance faster in <asterisk> + <literal> patterns
+ wildmatch: make a special case for "*/" with FNM_PATHNAME
+ test-wildmatch: add "perf" command to compare wildmatch and fnmatch
+ wildmatch: support "no FNM_PATHNAME" mode
+ wildmatch: make dowild() take arbitrary flags
+ wildmatch: rename constants and update prototype
Replace our use of fnmatch(3) with a more feature-rich wildmatch.
A handful patches at the bottom have been moved to nd/wildmatch to
graduate as part of that branch, before this series solidifies.
* rs/clarify-entry-cmp-sslice (2013-01-16) 1 commit
(merged to 'next' on 2013-01-18 at d584dc6)
+ refs: use strncmp() instead of strlen() and memcmp()
--------------------------------------------------
[New Topics]
* jc/push-reject-reasons (2013-01-24) 4 commits
- push: finishing touches to explain REJECT_ALREADY_EXISTS better
- push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE
- push: further simplify the logic to assign rejection reason
- push: further clean up fields of "struct ref"
Improve error and advice messages given locally when "git push"
refuses when it cannot compute fast-forwardness by separating these
cases from the normal "not a fast-forward; merge first and push
again" case.
Will merge to 'next'.
* as/test-cleanup (2013-01-24) 1 commit
- t7102 (reset): don't hardcode SHA-1 in expected outputs
Will merge to 'next'.
* jc/do-not-let-random-file-interfere-with-completion-tests (2013-01-24) 1 commit
- t9902: protect test from stray build artifacts
Scripts to test bash completion was inherently flaky as it was
affected by whatever random things the user may have on $PATH.
Will merge to 'next'.
* jk/cvsimport-does-not-work-with-cvsps3 (2013-01-24) 1 commit
- git-cvsimport.txt: cvsps-2 is deprecated
Warn people that other tools are more recommendable over
cvsimport+cvsps2 combo when doing a one-shot import, and cvsimport
will not work with cvsps3.
Will merge to 'next'.
* jk/mergetool (2013-01-25) 6 commits
- mergetool--lib: Improve show_tool_help() output
- mergetools/vim: Remove redundant diff command
- git-difftool: use git-mergetool--lib for "--tool-help"
- git-mergetool: don't hardcode 'mergetool' in show_tool_help
- git-mergetool: remove redundant assignment
- git-mergetool: move show_tool_help to mergetool--lib
Cleans up mergetool/difftool combo.
It seems John Keeping has some more updates but the version posted
breaks the tests rather badly.
Expecting some follow-ups.
* jn/do-not-drop-username-when-reading-from-etc-mailname (2013-01-25) 1 commit
- ident: do not drop username when reading from /etc/mailname
We used to stuff "user@" and then append what we read from
/etc/mailname to come up with a default e-mail ident, but a bug
lost the "user@" part. This is to fix it.
Will merge to 'next'.
* mm/add-u-A-sans-pathspec (2013-01-25) 1 commit
- add: warn when -u or -A is used without pathspec
Forbid "git add -u" and "git add -A" without pathspec run from a
subdirectory, to train people to type "." (or ":/") to make the
choice of default does not matter.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* mp/complete-paths (2013-01-11) 1 commit
- git-completion.bash: add support for path completion
The completion script used to let the default completer to suggest
pathnames, which gave too many irrelevant choices (e.g. "git add"
would not want to add an unmodified path). Teach it to use a more
git-aware logic to enumerate only relevant ones.
Waiting for area-experts' help and review.
* jl/submodule-deinit (2012-12-04) 1 commit
- submodule: add 'deinit' command
There was no Porcelain way to say "I no longer am interested in
this submodule", once you express your interest in a submodule with
"submodule init". "submodule deinit" is the way to do so.
Expecting a reroll.
$gmane/212884
* 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.
* rc/maint-complete-git-p4 (2012-09-24) 1 commit
- Teach git-completion about git p4
Comment from Pete will need to be addressed ($gmane/206172).
* 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/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.
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 reroll.
$gmane/210151
"The first remote becomes the default" bit is better done as a
separate step.
* nd/parse-pathspec (2013-01-11) 20 commits
. Convert more init_pathspec() to parse_pathspec()
. Convert add_files_to_cache to take struct pathspec
. Convert {read,fill}_directory to take struct pathspec
. Convert refresh_index to take struct pathspec
. Convert report_path_error to take struct pathspec
. checkout: convert read_tree_some to take struct pathspec
. Convert unmerge_cache to take struct pathspec
. Convert read_cache_preload() to take struct pathspec
. add: convert to use parse_pathspec
. archive: convert to use parse_pathspec
. ls-files: convert to use parse_pathspec
. rm: convert to use parse_pathspec
. checkout: convert to use parse_pathspec
. rerere: convert to use parse_pathspec
. status: convert to use parse_pathspec
. commit: convert to use parse_pathspec
. clean: convert to use parse_pathspec
. Export parse_pathspec() and convert some get_pathspec() calls
. Add parse_pathspec() that converts cmdline args to struct pathspec
. pathspec: save the non-wildcard length part
Uses the parsed pathspec structure in more places where we used to
use the raw "array of strings" pathspec.
Ejected from 'pu' for now; will take a look at the rerolled one
later ($gmane/213340).
--------------------------------------------------
[Cooking]
* bc/fix-array-syntax-for-3.0-in-completion-bash (2013-01-18) 1 commit
(merged to 'next' on 2013-01-25 at d113c1a)
+ git-completion.bash: replace zsh notation that breaks bash 3.X
Fix use of an array notation that older versions of bash do not
understand.
* jc/help (2013-01-18) 1 commit
(merged to 'next' on 2013-01-25 at b2b087e)
+ help: include <common-cmds.h> only in one file
A header file that has the definition of a static array was
included in two places, wasting the space.
* jc/hidden-refs (2013-01-18) 2 commits
- upload-pack: allow hiding ref hiearchies
- upload-pack: share more code
Allow the server side to unclutter the refs/ namespace it shows by
default, while still allowing requests for histories leading to the
tips of hidden refs by updated clients (which are not written yet).
* jk/update-install-for-p4 (2013-01-20) 1 commit
- INSTALL: git-p4 doesn't support Python 3
Will merge to 'next'.
* tb/t0050-maint (2013-01-21) 3 commits
(merged to 'next' on 2013-01-25 at 682b1e2)
+ t0050: Use TAB for indentation
+ t0050: honor CASE_INSENSITIVE_FS in add (with different case)
+ t0050: known breakage vanished in merge (case change)
Update tests that were expecting to fail due to a bug that was
fixed earlier.
* nd/magic-pathspec-from-root (2013-01-21) 2 commits
(merged to 'next' on 2013-01-25 at b056b57)
+ grep: avoid accepting ambiguous revision
+ Update :/abc ambiguity check
When giving arguments without "--" disambiguation, object names
that come earlier on the command line must not be interpretable as
pathspecs and pathspecs that come later on the command line must
not be interpretable as object names. Tweak the disambiguation
rule so that ":/" (no other string before or after) is always
interpreted as a pathspec, to avoid having to say "git cmd -- :/".
* ta/doc-no-small-caps (2013-01-22) 10 commits
- fixup! Change 'git' to 'Git' whenever the whole system is referred to #4
- Change 'git' to 'Git' whenever the whole system is referred to #4
- fixup! Change 'git' to 'Git' whenever the whole system is referred to #3
- Change 'git' to 'Git' whenever the whole system is referred to #3
- fixup! Change 'git' to 'Git' whenever the whole system is referred to #2
- Change 'git' to 'Git' whenever the whole system is referred to #2
- fixup! fixup! Change 'git' to 'Git' whenever the whole system is referred to #1
- fixup! Change 'git' to 'Git' whenever the whole system is referred to #1
- Change 'git' to 'Git' whenever the whole system is referred to #1
- Documentation: avoid poor-man's small caps
Update documentation to change "GIT" which was a poor-man's small
caps to "Git" which was the intended spelling. Also change "git"
spelled in all-lowercase to "Git" when it refers to the system as
the whole or the concept it embodies, as opposed to the command the
end users would type.
Will wait for a week or so (say, til end of January) for Thomas to
collect fix-ups, squash the result into two patches and then merge
to 'next'.
* rr/minimal-stat (2013-01-22) 1 commit
(merged to 'next' on 2013-01-25 at 11c4453)
+ Enable minimal stat checking
Some reimplementations of Git does not write all the stat info back
to the index due to their implementation limitations (e.g. jgit
running on Java). A configuration option can tell Git to ignore
changes to most of the stat fields and only pay attention to mtime
and size, which these implementations can reliably update. This
avoids excessive revalidation of contents.
* jc/remove-treesame-parent-in-simplify-merges (2013-01-17) 1 commit
- simplify-merges: drop merge from irrelevant side branch
The --simplify-merges logic did not cull irrelevant parents from a
merge that is otherwise not interesting with respect to the paths
we are following.
As this touches a fairly core part of the revision traversal
infrastructure, it is appreciated to have an extra set of eyes for
sanity check.
Waiting for reviews and comments.
* jk/remote-helpers-in-python-3 (2013-01-24) 8 commits
(merged to 'next' on 2013-01-25 at acf9419)
+ git-remote-testpy: call print as a function
+ git-remote-testpy: don't do unbuffered text I/O
+ git-remote-testpy: hash bytes explicitly
+ svn-fe: allow svnrdump_sim.py to run with Python 3
+ git_remote_helpers: use 2to3 if building with Python 3
+ git_remote_helpers: force rebuild if python version changes
+ git_remote_helpers: fix input when running under Python 3
+ git_remote_helpers: allow building with Python 3
Prepare remote-helper test written in Python to be run with Python3.
* dl/am-hg-locale (2013-01-18) 1 commit
(merged to 'next' on 2013-01-25 at 3419019)
+ am: invoke perl's strftime in C locale
Datestamp recorded in "Hg" format patch was reformatted incorrectly
to an e-mail looking date using locale dependant strftime, causing
patch application to fail.
* jk/config-parsing-cleanup (2013-01-23) 8 commits
- reflog: use parse_config_key in config callback
- help: use parse_config_key for man config
- submodule: simplify memory handling in config parsing
- submodule: use parse_config_key when parsing config
- userdiff: drop parse_driver function
- convert some config callbacks to parse_config_key
- archive-tar: use parse_config_key when parsing config
- config: add helper function for parsing key names
Configuration parsing for tar.* configuration variables were
broken. Introduce a new config-keyname parser API to make the
callers much less error prone.
Will merge to 'next'.
* mp/diff-algo-config (2013-01-16) 3 commits
- diff: Introduce --diff-algorithm command line option
- config: Introduce diff.algorithm variable
- git-completion.bash: Autocomplete --minimal and --histogram for git-diff
Add diff.algorithm configuration so that the user does not type
"diff --histogram".
Looking better; may want tests to protect it from future breakages,
but otherwise it looks ready for 'next'.
Expecting a follow-up to add tests.
* jc/custom-comment-char (2013-01-16) 1 commit
(merged to 'next' on 2013-01-25 at 91d8a5d)
+ Allow custom "comment char"
An illustration to show codepaths that need to be touched to change
the hint lines in the edited text to begin with something other
than '#'.
This is half my work and half by Ralf Thielow. There may still be
leftover '#' lurking around, though. My "git grep" says C code
should be already fine, but git-rebase--interactive.sh could be
converted (it should not matter, as the file is not really a
free-form text).
I don't know how useful this will be in real life, though.
* nd/fetch-depth-is-broken (2013-01-11) 3 commits
(merged to 'next' on 2013-01-15 at 70a5ca7)
+ fetch: elaborate --depth action
+ upload-pack: fix off-by-one depth calculation in shallow clone
+ fetch: add --unshallow for turning shallow repo into complete one
"git fetch --depth" was broken in at least three ways. The
resulting history was deeper than specified by one commit, it was
unclear how to wipe the shallowness of the repository with the
command, and documentation was misleading.
Will cook in 'next'.
* jc/no-git-config-in-clone (2013-01-11) 1 commit
(merged to 'next' on 2013-01-15 at feeffe1)
+ clone: do not export and unexport GIT_CONFIG
We stopped paying attention to $GIT_CONFIG environment that points
at a single configuration file from any command other than "git config"
quite a while ago, but "git clone" internally set, exported, and
then unexported the variable during its operation unnecessarily.
Will cook in 'next'.
* dg/subtree-fixes (2013-01-08) 7 commits
- contrib/subtree: mkdir the manual directory if needed
- contrib/subtree: honor $(DESTDIR)
- contrib/subtree: fix synopsis and command help
- contrib/subtree: better error handling for "add"
- contrib/subtree: add --unannotate option
- contrib/subtree: use %B for split Subject/Body
- t7900: remove test number comments
contrib/subtree updates; there are a few more from T. Zheng that
were posted separately, with an overlap.
Expecting a reroll.
* jc/push-2.0-default-to-simple (2013-01-16) 14 commits
(merged to 'next' on 2013-01-16 at 23f5df2)
+ t5570: do not assume the "matching" push is the default
+ t5551: do not assume the "matching" push is the default
+ t5550: do not assume the "matching" push is the default
(merged to 'next' on 2013-01-09 at 74c3498)
+ doc: push.default is no longer "matching"
+ push: switch default from "matching" to "simple"
+ t9401: do not assume the "matching" push is the default
+ t9400: do not assume the "matching" push is the default
+ t7406: do not assume the "matching" push is the default
+ t5531: do not assume the "matching" push is the default
+ t5519: do not assume the "matching" push is the default
+ t5517: do not assume the "matching" push is the default
+ t5516: do not assume the "matching" push is the default
+ t5505: do not assume the "matching" push is the default
+ t5404: do not assume the "matching" push is the default
Will cook in 'next' until Git 2.0 ;-).
* mb/gitweb-highlight-link-target (2012-12-20) 1 commit
- Highlight the link target line in Gitweb using CSS
Expecting a reroll.
$gmane/211935
* bc/append-signed-off-by (2013-01-21) 10 commits
- Unify appending signoff in format-patch, commit and sequencer
- format-patch: update append_signoff prototype
- t4014: more tests about appending s-o-b lines
- sequencer.c: teach append_signoff to avoid adding a duplicate newline
- sequencer.c: teach append_signoff how to detect duplicate s-o-b
- sequencer.c: always separate "(cherry picked from" from commit body
- sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
- t/t3511: add some tests of 'cherry-pick -s' functionality
- t/test-lib-functions.sh: allow to specify the tag name to test_commit
- sequencer.c: remove broken support for rfc2822 continuation in footer
Rerolled.
Expecting another reroll.
$gmane/214231
--------------------------------------------------
[Discarded]
* er/replace-cvsimport (2013-01-12) 7 commits
. t/lib-cvs: cvsimport no longer works without Python >= 2.7
. t9605: test for cvsps commit ordering bug
. t9604: fixup for new cvsimport
. t9600: fixup for new cvsimport
. t/lib-cvs.sh: allow cvsps version 3.x.
. t/t960[123]: remove leftover scripts
. cvsimport: rewrite to use cvsps 3.x to fix major bugs
Rerolled as jc/cvsimport-upgrade.
* jc/valgrind-memcmp-bsearch (2013-01-14) 1 commit
. ignore memcmp() overreading in bsearch() callback
Squelch false positive in valgrind tests; made unnecessary by
rewriting the callsite that confuses the tool.
* rs/archive-tar-config-parsing-fix (2013-01-14) 1 commit
. archive-tar: fix sanity check in config parsing
jk/config-parsing-cleanup made this topic unneeded.
* jc/cvsimport-upgrade (2013-01-14) 8 commits
- t9600: adjust for new cvsimport
- t9600: further prepare for sharing
- cvsimport-3: add a sample test
- cvsimport: make tests reusable for cvsimport-3
- cvsimport: start adding cvsps 3.x support
- cvsimport: introduce a version-switch wrapper
- cvsimport: allow setting a custom cvsps (2.x) program name
- Makefile: add description on PERL/PYTHON_PATH
People involved in cvsimport-3 say that the combination of it and
cvsps3 does not work with incremental import all that well. For
now, let's discard this and replace it with a documeentation update
that says "cvsimport will not work if you do not have cvsps2".
Will discard.
^ permalink raw reply
* Re: [PATCH 8/7] mergetool--lib: don't call "exit" in setup_tool
From: Junio C Hamano @ 2013-01-26 0:24 UTC (permalink / raw)
To: John Keeping; +Cc: David Aguilar, git
In-Reply-To: <20130125220359.GF7498@serenity.lan>
Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break
t7610 rather badly.
--- >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ---
...
ok 1 - setup
expecting success:
git checkout -b test1 branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
( yes "" | git mergetool both >/dev/null 2>&1 ) &&
( yes "" | git mergetool file1 file1 ) &&
( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&
test "$(cat file1)" = "master updated" &&
test "$(cat file2)" = "master new" &&
test "$(cat subdir/file3)" = "master new sub" &&
test "$(cat submod/bar)" = "branch1 submodule" &&
git commit -m "branch1 resolved with mergetool"
M submod
Switched to a new branch 'test1'
Submodule path 'submod': checked out '39c7f044ed2e6a9cebd5266529badd181c8762b5'
not ok - 2 custom mergetool
#
# git checkout -b test1 branch1 &&
# git submodule update -N &&
# test_must_fail git merge master >/dev/null 2>&1 &&
# ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
# ( yes "" | git mergetool file1 file1 ) &&
# ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
# ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
# ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
# ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
# ( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&
# test "$(cat file1)" = "master updated" &&
# test "$(cat file2)" = "master new" &&
# test "$(cat subdir/file3)" = "master new sub" &&
# test "$(cat submod/bar)" = "branch1 submodule" &&
# git commit -m "branch1 resolved with mergetool"
#
--- 8< ------ 8< ------ 8< ------ 8< ------ 8< ------ 8< ---
Due to ">dev/null 2>&1", all of the error clues are hidden, and I
didn't dig further to see which one was failing (this is why tests
shouldn't do these in general).
^ permalink raw reply
* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Eric Blake @ 2013-01-26 0:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Mark Levedahl, Ramsay Jones, Jonathan Nieder, Alex Riesen,
Jason Pyeron, git, Torsten Bögershausen,
Stephen & Linda Smith
In-Reply-To: <7v38xo3irh.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]
On 01/25/2013 05:11 PM, Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> Cygwin and Windows should be treated as completely separate platforms:
>> if __CYGWIN__ is defined, do one thing, if not, go ahead and check
>> WIN32, but the WIN32 macro should never be tested once we know the
>> platform is CYGWIN - these really are different platforms (if you are
>> unsure of this, consider that Cygwin includes a cross-compiler to
>> target native Win32 as the Cygwin maintainers recognized the platforms
>> are different).
>
> Not disagreeing with your conclusion (they should be treated as
> different), why does it define WIN32 in the first place?
>
> Perhaps we would want
>
> #ifdef __CYGWIN__
> #undef WIN32
> #endif
Wouldn't work. Cygwin gcc does NOT define WIN32; rather, the inclusion
of a Windows system header (generally discouraged, but sometimes a
necessary evil) might cause WIN32 to be defined for all subsequent headers.
Which is why other projects, like gnulib, have
# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
all over the place.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply
* [PATCH v2 0/2] git-web--browser: avoid errors in terminal when running
From: Alexey Shumkin @ 2013-01-26 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Alexey Shumkin, git
In-Reply-To: <7v1ud93uw8.fsf@alter.siamese.dyndns.org>
Reroll patch after all suggestions
Alexey Shumkin (2):
t9901-git-web--browse.sh: Use "write_script" helper
git-web--browser: avoid errors in terminal when running Firefox on
Windows
git-web--browse.sh | 2 +-
t/t9901-git-web--browse.sh | 59 ++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 55 insertions(+), 6 deletions(-)
--
1.8.1.1.10.g71fa0b7
^ permalink raw reply
* [PATCH v2 2/2] git-web--browser: avoid errors in terminal when running Firefox on Windows
From: Alexey Shumkin @ 2013-01-26 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Alexey Shumkin, git
In-Reply-To: <cover.1359160531.git.Alex.Crezoff@gmail.com>
Firefox on Windows by default is placed in "C:\Program Files\Mozilla Firefox"
folder, i.e. its path contains spaces. Before running this browser "git-web--browse"
tests version of Firefox to decide whether to use "-new-tab" option or not.
Quote browser path to avoid error during this test.
Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
---
git-web--browse.sh | 2 +-
t/t9901-git-web--browse.sh | 53 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/git-web--browse.sh b/git-web--browse.sh
index 1e82726..f96e5bd 100755
--- a/git-web--browse.sh
+++ b/git-web--browse.sh
@@ -149,7 +149,7 @@ fi
case "$browser" in
firefox|iceweasel|seamonkey|iceape)
# Check version because firefox < 2.0 does not support "-new-tab".
- vers=$(expr "$($browser_path -version)" : '.* \([0-9][0-9]*\)\..*')
+ vers=$(expr "$("$browser_path" -version)" : '.* \([0-9][0-9]*\)\..*')
NEWTAB='-new-tab'
test "$vers" -lt 2 && NEWTAB=''
"$browser_path" $NEWTAB "$@" &
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index b0dabf7..c1ee813 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -8,8 +8,23 @@ This test checks that git web--browse can handle various valid URLs.'
. ./test-lib.sh
test_web_browse () {
- # browser=$1 url=$2
+ # browser=$1 url=$2 sleep_timeout=$3
+ sleep_timeout="$3"
+ rm -f fake_browser_ran &&
git web--browse --browser="$1" "$2" >actual &&
+ # if $3 is set
+ # as far as Firefox is run in background (it is run with &)
+ # we trying to avoid race condition
+ # by waiting for "$sleep_timeout" seconds of timeout for 'fake_browser_ran' file appearance
+ if test -n "$sleep_timeout"
+ then
+ for timeout in $(test_seq $sleep_timeout)
+ do
+ test -f fake_browser_ran && break
+ sleep 1
+ done
+ test $timeout -ne $sleep_timeout
+ fi &&
tr -d '\015' <actual >text &&
test_cmp expect text
}
@@ -46,6 +61,42 @@ test_expect_success \
'
test_expect_success \
+ 'Paths are properly quoted for Firefox. Version older then v2.0' '
+ echo "fake: http://example.com/foo" >expect &&
+ write_script "fake browser" <<-\EOF &&
+
+ if test "$1" = "-version"; then
+ echo "Fake Firefox browser version 1.2.3"
+ else
+ # Firefox (in contrast to w3m) is run in background (with &)
+ # so redirect output to "actual"
+ echo "fake: ""$@" >actual
+ fi
+ : >fake_browser_ran
+ EOF
+ git config browser.firefox.path "$(pwd)/fake browser" &&
+ test_web_browse firefox http://example.com/foo 5
+'
+
+test_expect_success \
+ 'Paths are properly quoted for Firefox. Version v2.0 and above' '
+ echo "fake: -new-tab http://example.com/foo" >expect &&
+ write_script "fake browser" <<-\EOF &&
+
+ if test "$1" = "-version"; then
+ echo "Fake Firefox browser version 2.0.0"
+ else
+ # Firefox (in contrast to w3m) is run in background (with &)
+ # so redirect output to "actual"
+ echo "fake: ""$@" >actual
+ fi
+ : >fake_browser_ran
+ EOF
+ git config browser.firefox.path "$(pwd)/fake browser" &&
+ test_web_browse firefox http://example.com/foo 5
+'
+
+test_expect_success \
'browser command allows arbitrary shell code' '
echo "arg: http://example.com/foo" >expect &&
git config browser.custom.cmd "
--
1.8.1.1.10.g71fa0b7
^ permalink raw reply related
* [PATCH v2 1/2] t9901-git-web--browse.sh: Use "write_script" helper
From: Alexey Shumkin @ 2013-01-26 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Alexey Shumkin, git
In-Reply-To: <cover.1359160531.git.Alex.Crezoff@gmail.com>
Use "write_script" helper as suggested by Junio C Hamano.
Also, replace `pwd` with $(pwd) call convention.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
---
t/t9901-git-web--browse.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh
index b0a6bad..b0dabf7 100755
--- a/t/t9901-git-web--browse.sh
+++ b/t/t9901-git-web--browse.sh
@@ -38,12 +38,10 @@ test_expect_success \
test_expect_success \
'browser paths are properly quoted' '
echo fake: http://example.com/foo >expect &&
- cat >"fake browser" <<-\EOF &&
- #!/bin/sh
+ write_script "fake browser" <<-\EOF &&
echo fake: "$@"
EOF
- chmod +x "fake browser" &&
- git config browser.w3m.path "`pwd`/fake browser" &&
+ git config browser.w3m.path "$(pwd)/fake browser" &&
test_web_browse w3m http://example.com/foo
'
--
1.8.1.1.10.g71fa0b7
^ permalink raw reply related
* Re: [PATCH] mergetools: Enhance tortoisemerge to work with
From: Sven Strickroth @ 2013-01-26 0:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Aguilar, Sebastian Schuberth, Jeff King
In-Reply-To: <7v622l5d87.fsf@alter.siamese.dyndns.org>
Am 25.01.2013 19:28 schrieb Junio C Hamano:> Sven Strickroth
<sven.strickroth@tu-clausthal.de> writes:
>
>> TortoiseGitMerge and filenames with spaces
>
> ??? ECANNOTPARSE.
>
> ... ah, wait. Is this a broken-off tail of your subject line?
Yes.
>> + touch "$BACKUP"
>> + basename="$(basename "$merge_tool_path" .exe)"
>> + if test "$basename" = "tortoisegitmerge"
>> + then
>> + "$merge_tool_path" \
>> + -base "$BASE" -mine "$LOCAL" \
>> + -theirs "$REMOTE" -merged "$MERGED"
>> + else
>> + "$merge_tool_path" \
>> + -base:"$BASE" -mine:"$LOCAL" \
>> + -theirs:"$REMOTE" -merged:"$MERGED"
>
> Hmph.
>
> How was the support for "names with spaces" added in this new code?
> I do not spot what is different between this "else" clause and the
> original body of the merge_cmd (which only supported tortoisemerge).
>
> They seem to be doing exactly the same thing.
Erhm, no. As already stated and also mentioned in the commit log:
TortoiseMerge has cli parameter key-values separated by colons,
TortoiseGitMerge has key-values separated by spaces.
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply
* [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
From: Jonathan Nieder @ 2013-01-26 1:03 UTC (permalink / raw)
To: Eric Blake
Cc: Junio C Hamano, Mark Levedahl, Ramsay Jones, Alex Riesen,
Jason Pyeron, git, Torsten Bögershausen,
Stephen & Linda Smith
In-Reply-To: <51032481.4030707@redhat.com>
Throughout git, it is assumed that the WIN32 preprocessor symbol is
defined on native Windows setups (mingw and msvc) and not on Cygwin.
On Cygwin, most of the time git can pretend this is just another Unix
machine, and Windows-specific magic is generally counterproductive.
Unfortunately Cygwin *does* define the WIN32 symbol in some headers.
Best to rely on a new git-specific symbol NATIVE_WINDOWS instead,
defined as follows:
#if defined(WIN32) && !defined(__CYGWIN__)
# define NATIVE_WINDOWS
#endif
After this change, it should be possible to drop the
CYGWIN_V15_WIN32API setting without any negative effect.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Eric Blake wrote:
> Which is why other projects, like gnulib, have
>
> # if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>
> all over the place.
So, how about this?
Completely untested.
abspath.c | 2 +-
compat/terminal.c | 4 ++--
compat/win32.h | 2 +-
diff-no-index.c | 2 +-
git-compat-util.h | 3 ++-
help.c | 2 +-
run-command.c | 10 +++++-----
test-chmtime.c | 2 +-
thread-utils.c | 2 +-
9 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -3,7 +3,7 @@
#include "sigchain.h"
#include "strbuf.h"
-#if defined(HAVE_DEV_TTY) || defined(WIN32)
+#if defined(HAVE_DEV_TTY) || defined(WINDOWS_NATIVE)
static void restore_term(void);
@@ -53,7 +53,7 @@ error:
return -1;
}
-#elif defined(WIN32)
+#elif defined(WINDOWS_NATIVE)
#define INPUT_PATH "CONIN$"
#define OUTPUT_PATH "CONOUT$"
diff --git a/compat/win32.h b/compat/win32.h
index 8ce91048..31dd30f7 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -2,7 +2,7 @@
#define WIN32_H
/* common Win32 functions for MinGW and Cygwin */
-#ifndef WIN32 /* Not defined by Cygwin */
+#ifndef WINDOWS_NATIVE /* Not defined for Cygwin */
#include <windows.h>
#endif
diff --git a/diff-no-index.c b/diff-no-index.c
index 74da6593..a0bc9c50 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -45,7 +45,7 @@ static int get_mode(const char *path, int *mode)
if (!path || !strcmp(path, "/dev/null"))
*mode = 0;
-#ifdef _WIN32
+#ifdef WINDOWS_NATIVE
else if (!strcasecmp(path, "nul"))
*mode = 0;
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b745..ebbdff53 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,10 +85,11 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
+#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
#include <winsock2.h>
#include <windows.h>
+#define WINDOWS_NATIVE
#endif
#include <unistd.h>
diff --git a/help.c b/help.c
index 2a42ec6d..cc1e63f7 100644
--- a/help.c
+++ b/help.c
@@ -106,7 +106,7 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
+#if defined(WINDOWS_NATIVE) || defined(__CYGWIN__)
#if defined(__CYGWIN__)
if ((st.st_mode & S_IXUSR) == 0)
#endif
diff --git a/run-command.c b/run-command.c
index 04712191..04ac6181 100644
--- a/run-command.c
+++ b/run-command.c
@@ -72,7 +72,7 @@ static inline void close_pair(int fd[2])
close(fd[1]);
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static inline void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
@@ -159,7 +159,7 @@ static const char **prepare_shell_cmd(const char **argv)
die("BUG: shell command is empty");
if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
nargv[nargc++] = SHELL_PATH;
#else
nargv[nargc++] = "sh";
@@ -182,7 +182,7 @@ static const char **prepare_shell_cmd(const char **argv)
return nargv;
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int execv_shell_cmd(const char **argv)
{
const char **nargv = prepare_shell_cmd(argv);
@@ -193,7 +193,7 @@ static int execv_shell_cmd(const char **argv)
}
#endif
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int child_err = 2;
static int child_notifier = -1;
@@ -330,7 +330,7 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
fflush(NULL);
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
{
int notify_pipe[2];
if (pipe(notify_pipe))
diff --git a/test-chmtime.c b/test-chmtime.c
index 92713d16..803b6055 100644
--- a/test-chmtime.c
+++ b/test-chmtime.c
@@ -87,7 +87,7 @@ int main(int argc, const char *argv[])
return -1;
}
-#ifdef WIN32
+#ifdef WINDOWS_NATIVE
if (!(sb.st_mode & S_IWUSR) &&
chmod(argv[i], sb.st_mode | S_IWUSR)) {
fprintf(stderr, "Could not make user-writable %s: %s",
diff --git a/thread-utils.c b/thread-utils.c
index 7f4b76a9..4c4cf2fa 100644
--- a/thread-utils.c
+++ b/thread-utils.c
@@ -24,7 +24,7 @@ int online_cpus(void)
long ncpus;
#endif
-#ifdef _WIN32
+#ifdef WINDOWS_NATIVE
SYSTEM_INFO info;
GetSystemInfo(&info);
--
1.8.1.1
^ permalink raw reply related
* Re: [PATCH] mergetools: Enhance tortoisemerge to work with
From: Sven Strickroth @ 2013-01-26 1:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Aguilar, Sebastian Schuberth, Jeff King
In-Reply-To: <7v622l5d87.fsf@alter.siamese.dyndns.org>
The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
(starting with 1.8.0) in order to make clear that this one has special
support for git and prevent confusion with the TortoiseSVN TortoiseMerge
version.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
mergetools/tortoisemerge | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..8476afa 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -11,7 +11,16 @@ merge_cmd () {
-theirs:"$REMOTE" -merged:"$MERGED"
check_unchanged
else
- echo "TortoiseMerge cannot be used without a base" 1>&2
+ echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
fi
}
+
+translate_merge_tool_path() {
+ if type tortoisegitmerge >/dev/null 2>/dev/null
+ then
+ echo tortoisegitmerge
+ else
+ echo tortoisemerge
+ fi
+}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 1/2] mergetools: Added support for TortoiseGitMerge
From: Sven Strickroth @ 2013-01-26 1:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Aguilar, Sebastian Schuberth, Jeff King
In-Reply-To: <7v622l5d87.fsf@alter.siamese.dyndns.org>
The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
(starting with 1.8.0) in order to make clear that this one has special
support for git and prevent confusion with the TortoiseSVN TortoiseMerge
version.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
mergetools/tortoisemerge | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index ed7db49..8476afa 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -11,7 +11,16 @@ merge_cmd () {
-theirs:"$REMOTE" -merged:"$MERGED"
check_unchanged
else
- echo "TortoiseMerge cannot be used without a base" 1>&2
+ echo "$merge_tool_path cannot be used without a base" 1>&2
return 1
fi
}
+
+translate_merge_tool_path() {
+ if type tortoisegitmerge >/dev/null 2>/dev/null
+ then
+ echo tortoisegitmerge
+ else
+ echo tortoisemerge
+ fi
+}
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* [PATCH 2/2] mergetools: Make tortoisemerge work with
From: Sven Strickroth @ 2013-01-26 1:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Aguilar, Sebastian Schuberth, Jeff King
In-Reply-To: <7v622l5d87.fsf@alter.siamese.dyndns.org>
tortoisegitmerge and filesnames with space
The tortoisemerge mergetool does not work with filenames which have
a space in it. Fixing this required changes in git and also in
TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.
TortoiseGitMerge now separates cli parameter key-values by space instead
of colons as TortoiseSVN TortoiseMerge does and supports filesnames
with spaces in it this way now.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
Reported-by: Sebastian Schuberth <sschuberth@gmail.com>
---
mergetools/tortoisemerge | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
index 8476afa..2f98829 100644
--- a/mergetools/tortoisemerge
+++ b/mergetools/tortoisemerge
@@ -6,9 +6,17 @@ merge_cmd () {
if $base_present
then
touch "$BACKUP"
- "$merge_tool_path" \
- -base:"$BASE" -mine:"$LOCAL" \
- -theirs:"$REMOTE" -merged:"$MERGED"
+ basename="$(basename "$merge_tool_path" .exe)"
+ if test "$basename" = "tortoisegitmerge"
+ then
+ "$merge_tool_path" \
+ -base "$BASE" -mine "$LOCAL" \
+ -theirs "$REMOTE" -merged "$MERGED"
+ else
+ "$merge_tool_path" \
+ -base:"$BASE" -mine:"$LOCAL" \
+ -theirs:"$REMOTE" -merged:"$MERGED"
+ fi
check_unchanged
else
echo "$merge_tool_path cannot be used without a base" 1>&2
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* Re: [PATCH v2 1/3] branch: reject -D/-d without branch name
From: Duy Nguyen @ 2013-01-26 3:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matthieu Moy
In-Reply-To: <7vham53x01.fsf@alter.siamese.dyndns.org>
On Sat, Jan 26, 2013 at 2:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> ---
>> builtin/branch.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Forgot to sign-off?
Yes
> Is this a real problem?
Yes. My thoughts yesterday when I happened to type "git branch -D":
"Wait, I just entered a delete command without a branch name. It did
not report anything, which in UNIX world usually means successful
operation. _What_ did it delete??"
I knew this code so I just went check. If I did not know, I might as
well list all branches I had and see if something was missing. A short
message would have saved me the trouble.
> I do not see it particularly wrong to succeed after deleting 0 or
> more given branch names.
--
Duy
^ permalink raw reply
* [PATCH] mergetools: Simplify how we handle "vim" and "defaults"
From: David Aguilar @ 2013-01-26 6:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
Remove the exceptions for "vim" and "defaults" in the mergetool library
so that every filename in mergetools/ matches 1:1 with the name of a
valid built-in tool.
Make common functions available in $MERGE_TOOLS_DIR/include/.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
This should make things ok on platforms where we don't have symlinks.
Makefile | 2 +-
git-mergetool--lib.sh | 41 +++++++++++-----------------
mergetools/gvimdiff | 1 +
mergetools/gvimdiff2 | 1 +
mergetools/{defaults => include/defaults.sh} | 0
mergetools/{vim => include/vim.sh} | 0
mergetools/vimdiff | 1 +
mergetools/vimdiff2 | 1 +
8 files changed, 21 insertions(+), 26 deletions(-)
create mode 100644 mergetools/gvimdiff
create mode 100644 mergetools/gvimdiff2
rename mergetools/{defaults => include/defaults.sh} (100%)
rename mergetools/{vim => include/vim.sh} (100%)
create mode 100644 mergetools/vimdiff
create mode 100644 mergetools/vimdiff2
diff --git a/Makefile b/Makefile
index f69979e..3bc6eb5 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,7 +2724,7 @@ install: all
$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
- $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+ cp -R mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
ifndef NO_GETTEXT
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
(cd po/build/locale && $(TAR) cf - .) | \
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index aa38bd1..c866ed8 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,7 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
+MERGE_TOOLS_DIR="$(git --exec-path)/mergetools"
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -44,25 +46,14 @@ valid_tool () {
}
setup_tool () {
- case "$1" in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool="$1"
- ;;
- esac
- mergetools="$(git --exec-path)/mergetools"
+ tool="$1"
- # Load the default definitions
- . "$mergetools/defaults"
- if ! test -f "$mergetools/$tool"
+ if ! test -f "$MERGE_TOOLS_DIR/$tool"
then
return 1
fi
-
- # Load the redefined functions
- . "$mergetools/$tool"
+ . "$MERGE_TOOLS_DIR/include/defaults.sh"
+ . "$MERGE_TOOLS_DIR/$tool"
if merge_mode && ! can_merge
then
@@ -99,7 +90,7 @@ run_merge_tool () {
base_present="$2"
status=0
- # Bring tool-specific functions into scope
+ # Bring tool specific functions into scope
setup_tool "$1"
if merge_mode
@@ -177,18 +168,17 @@ list_merge_tool_candidates () {
show_tool_help () {
unavailable= available= LF='
'
-
- scriptlets="$(git --exec-path)"/mergetools
- for i in "$scriptlets"/*
+ for i in "$MERGE_TOOLS_DIR"/*
do
- . "$scriptlets"/defaults
- . "$i"
-
- tool="$(basename "$i")"
- if test "$tool" = "defaults"
+ if test -d "$i"
then
continue
- elif merge_mode && ! can_merge
+ fi
+
+ . "$MERGE_TOOLS_DIR"/include/defaults.sh
+ . "$i"
+
+ if merge_mode && ! can_merge
then
continue
elif diff_mode && ! can_diff
@@ -196,6 +186,7 @@ show_tool_help () {
continue
fi
+ tool=$(basename "$i")
merge_tool_path=$(translate_merge_tool_path "$tool")
if type "$merge_tool_path" >/dev/null 2>&1
then
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 100644
index 0000000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 100644
index 0000000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
diff --git a/mergetools/defaults b/mergetools/include/defaults.sh
similarity index 100%
rename from mergetools/defaults
rename to mergetools/include/defaults.sh
diff --git a/mergetools/vim b/mergetools/include/vim.sh
similarity index 100%
rename from mergetools/vim
rename to mergetools/include/vim.sh
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
new file mode 100644
index 0000000..f5890b1
--- /dev/null
+++ b/mergetools/vimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
new file mode 100644
index 0000000..f5890b1
--- /dev/null
+++ b/mergetools/vimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/include/vim.sh"
--
1.8.0.6.ge6f188f.dirty
^ permalink raw reply related
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Torsten Bögershausen @ 2013-01-26 6:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, Jonathan Nieder, git, kraai
In-Reply-To: <7vk3re2ncb.fsf@alter.siamese.dyndns.org>
On 15.01.13 21:38, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> What do we think about something like this for fishing for which:
>>
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -644,6 +644,10 @@ yes () {
>> :
>> done
>> }
>> +which () {
>> + echo >&2 "which is not portable (please use type)"
>> + exit 1
>> +}
>>
>>
>> This will happen in runtime, which might be good enough ?
>
> if (
> which frotz &&
> test $(frobonitz --version" -le 2.0
> )
> then
> test_set_prereq FROTZ_FROBONITZ
> else
> echo >&2 "suitable Frotz/Frobonitz combo not available;"
> echo >&2 "some tests may be skipped"
> fi
>
> I somehow think this is a lost cause.
I found different ways to detect if frotz is installed in the test suite:
a) use "type" (Should be the fastest ?)
b) call the command directly, check the exit code
c) "! test_have_prereq" (easy to understand, propably most expensive ?)
Do we really need "which" to detect if frotz is installed?
/Torsten
=============
if ! type cvs >/dev/null 2>&1
then
skip_all='skipping cvsimport tests, cvs not found'
test_done
fi
===========
if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then
# we are in full-on bash mode
true
elif type bash >/dev/null 2>&1; then
# execute in full-on bash mode
unset POSIXLY_CORRECT
exec bash "$0" "$@"
else
echo '1..0 #SKIP skipping bash completion tests; bash not available'
exit 0
fi
===============
svn >/dev/null 2>&1
if test $? -ne 1
then
skip_all='skipping git svn tests, svn not found'
test_done
fi
===============
( p4 -h && p4d -h ) >/dev/null 2>&1 || {
skip_all='skipping git p4 tests; no p4 or p4d'
test_done
}
===============
if ! test_have_prereq PERL; then
skip_all='skipping gitweb tests, perl not available'
test_done
fi
^ permalink raw reply
* Re: [PATCH 8/7] mergetool--lib: don't call "exit" in setup_tool
From: David Aguilar @ 2013-01-26 7:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Keeping, git
In-Reply-To: <7vip6k23mk.fsf@alter.siamese.dyndns.org>
On Fri, Jan 25, 2013 at 4:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break
> t7610 rather badly.
I just sent a replacement for the vim/symlink issue stuff.
I tried to keep the patch small. John, can you rebase this
patch on top of it?
> --- >8 ------ >8 ------ >8 ------ >8 ------ >8 ------ >8 ---
> ...
> ok 1 - setup
>
> expecting success:
> git checkout -b test1 branch1 &&
> git submodule update -N &&
> test_must_fail git merge master >/dev/null 2>&1 &&
> ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool file1 file1 ) &&
> ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
> ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
> ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
> ( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&
> test "$(cat file1)" = "master updated" &&
> test "$(cat file2)" = "master new" &&
> test "$(cat subdir/file3)" = "master new sub" &&
> test "$(cat submod/bar)" = "branch1 submodule" &&
> git commit -m "branch1 resolved with mergetool"
>
> M submod
> Switched to a new branch 'test1'
> Submodule path 'submod': checked out '39c7f044ed2e6a9cebd5266529badd181c8762b5'
> not ok - 2 custom mergetool
> #
> # git checkout -b test1 branch1 &&
> # git submodule update -N &&
> # test_must_fail git merge master >/dev/null 2>&1 &&
> # ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
> # ( yes "" | git mergetool file1 file1 ) &&
> # ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
> # ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
> # ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
> # ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
> # ( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&
> # test "$(cat file1)" = "master updated" &&
> # test "$(cat file2)" = "master new" &&
> # test "$(cat subdir/file3)" = "master new sub" &&
> # test "$(cat submod/bar)" = "branch1 submodule" &&
> # git commit -m "branch1 resolved with mergetool"
> #
> --- 8< ------ 8< ------ 8< ------ 8< ------ 8< ------ 8< ---
>
> Due to ">dev/null 2>&1", all of the error clues are hidden, and I
> didn't dig further to see which one was failing (this is why tests
> shouldn't do these in general).
--
David
^ permalink raw reply
* Re: [PATCH 2/2] mergetools: Make tortoisemerge work with
From: David Aguilar @ 2013-01-26 7:10 UTC (permalink / raw)
To: Sven Strickroth; +Cc: Junio C Hamano, git, Sebastian Schuberth, Jeff King
In-Reply-To: <51032E96.2040209@tu-clausthal.de>
On Fri, Jan 25, 2013 at 5:17 PM, Sven Strickroth
<sven.strickroth@tu-clausthal.de> wrote:
> TortoiseGitMerge now separates cli parameter key-values by space instead
> of colons as TortoiseSVN TortoiseMerge does and supports filesnames
> with spaces in it this way now.
These patches look correct (I do not have the tool to test)
but I think we should fixup this commit message.
How about something like...
mergetools: Teach tortoisemerge about TortoiseGitMerge
TortoiseGitMerge improved its syntax to allow for file paths
with spaces. Detect when it is installed and prefer it over
TortoiseMerge.
--
David
^ permalink raw reply
* [PATCH 0/3] lazily load commit->buffer
From: Jeff King @ 2013-01-26 9:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathon Mah, Jonathan Nieder, Duy Nguyen, Stefan Näwe,
Armin, git@vger.kernel.org
In-Reply-To: <7vip6l5l71.fsf@alter.siamese.dyndns.org>
On Fri, Jan 25, 2013 at 07:36:18AM -0800, Junio C Hamano wrote:
> Jonathon Mah <jmah@me.com> writes:
>
> > Just to note, the proposals so far don't prevent a "smart-ass"
> > function from freeing the buffer when it's called underneath the
> > use/release scope, as in:
> >
> > with_commit_buffer(commit); {
> > fn1_needing_buffer(commit);
> > walk_rev_tree_or_something();
> > fn2_needing_buffer(commit);
> > } done_with_commit_buffer(commit);
>
> I think the goal of everybody discussing these ideas is to make sure
> that all code follows the simple ownership policy proposed at the
> beginning of this subthread: commit->buffer belongs to the revision
> traversal machinery, and other users could borrow it when available.
Yeah, agreed. I started to fix this up with a use/unuse pattern and
realized something: all of the call sites are calling logmsg_reencode
anyway, because that is the next logical step in doing anything with the
buffer that is not just parsing out the parent/timestamp/tree info. And
since that function already might allocate (for the re-encoded copy),
callers have to handle the maybe-borrowed-maybe-free situation already.
So I came up with this patch series, which I think should fix the
problem, and actually makes the call-sites easier to read, rather than
harder.
[1/3]: commit: drop useless xstrdup of commit message
[2/3]: logmsg_reencode: never return NULL
[3/3]: logmsg_reencode: lazily load missing commit buffers
Here's the diffstat:
builtin/blame.c | 22 ++-------
builtin/commit.c | 14 +-----
commit.h | 1 +
pretty.c | 93 ++++++++++++++++++++++++++---------
t/t4042-diff-textconv-caching.sh | 8 +++
5 files changed, 85 insertions(+), 53 deletions(-)
Not too bad, and 27 of the lines added in pretty.c are new comments
explaining the flow of logmsg_reencode. So even if this doesn't get
every case, I think it's a nice cleanup.
-Peff
^ permalink raw reply
* [PATCH 1/3] commit: drop useless xstrdup of commit message
From: Jeff King @ 2013-01-26 9:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathon Mah, Jonathan Nieder, Duy Nguyen, Stefan Näwe,
Armin, git@vger.kernel.org
In-Reply-To: <20130126094026.GA9646@sigill.intra.peff.net>
When git-commit is asked to reuse a commit message via "-c",
we call read_commit_message, which looks up the commit and
hands back either the re-encoded result, or a copy of the
original. We make a copy in the latter case so that the
ownership semantics of the return value are clear (in either
case, it can be freed).
However, since we return a "const char *", and since the
resulting buffer's lifetime is the same as that of the whole
program, we never bother to free it at all.
Let's just drop the copy. That saves us a copy in the common
case. While it does mean we leak in the re-encode case, it
doesn't matter, since we are relying on program exit to free
the memory anyway.
Signed-off-by: Jeff King <peff@peff.net>
---
This one isn't strictly necessary, but it makes it a lot more obvious
what is going on with the memory ownership of this code in the next
patch.
builtin/commit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 38b9a9c..fbbb40f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -962,7 +962,7 @@ static const char *read_commit_message(const char *name)
* encodings are identical.
*/
if (out == NULL)
- out = xstrdup(commit->buffer);
+ out = commit->buffer;
return out;
}
--
1.8.0.2.16.g72e2fc9
^ permalink raw reply related
* [PATCH 2/3] logmsg_reencode: never return NULL
From: Jeff King @ 2013-01-26 9:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathon Mah, Jonathan Nieder, Duy Nguyen, Stefan Näwe,
Armin, git@vger.kernel.org
In-Reply-To: <20130126094026.GA9646@sigill.intra.peff.net>
The logmsg_reencode function will return the reencoded
commit buffer, or NULL if reencoding failed or no reencoding
was necessary. Since every caller then ends up checking for NULL
and just using the commit's original buffer, anyway, we can
be a bit more helpful and just return that buffer when we
would have returned NULL.
Since the resulting string may or may not need to be freed,
we introduce a logmsg_free, which checks whether the buffer
came from the commit object or not (callers either
implemented the same check already, or kept two separate
pointers, one to mark the buffer to be used, and one for the
to-be-freed string).
Pushing this logic into logmsg_* simplifies the callers, and
will let future patches lazily load the commit buffer in a
single place.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/blame.c | 9 ++++-----
builtin/commit.c | 14 ++------------
commit.h | 1 +
pretty.c | 38 ++++++++++++++++++++++----------------
4 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index b431ba3..962e4e3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1420,7 +1420,7 @@ static void get_commit_info(struct commit *commit,
{
int len;
const char *subject, *encoding;
- char *reencoded, *message;
+ char *message;
commit_info_init(ret);
@@ -1438,14 +1438,13 @@ static void get_commit_info(struct commit *commit,
sha1_to_hex(commit->object.sha1));
}
encoding = get_log_output_encoding();
- reencoded = logmsg_reencode(commit, encoding);
- message = reencoded ? reencoded : commit->buffer;
+ message = logmsg_reencode(commit, encoding);
get_ac_line(message, "\nauthor ",
&ret->author, &ret->author_mail,
&ret->author_time, &ret->author_tz);
if (!detailed) {
- free(reencoded);
+ logmsg_free(message, commit);
return;
}
@@ -1459,7 +1458,7 @@ static void get_commit_info(struct commit *commit,
else
strbuf_addf(&ret->summary, "(%s)", sha1_to_hex(commit->object.sha1));
- free(reencoded);
+ logmsg_free(message, commit);
}
/*
diff --git a/builtin/commit.c b/builtin/commit.c
index fbbb40f..6169f1e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -946,24 +946,14 @@ static const char *read_commit_message(const char *name)
static const char *read_commit_message(const char *name)
{
- const char *out_enc, *out;
+ const char *out_enc;
struct commit *commit;
commit = lookup_commit_reference_by_name(name);
if (!commit)
die(_("could not lookup commit %s"), name);
out_enc = get_commit_output_encoding();
- out = logmsg_reencode(commit, out_enc);
-
- /*
- * If we failed to reencode the buffer, just copy it
- * byte for byte so the user can try to fix it up.
- * This also handles the case where input and output
- * encodings are identical.
- */
- if (out == NULL)
- out = commit->buffer;
- return out;
+ return logmsg_reencode(commit, out_enc);
}
static int parse_and_validate_options(int argc, const char *argv[],
diff --git a/commit.h b/commit.h
index c16c8a7..e770649 100644
--- a/commit.h
+++ b/commit.h
@@ -101,6 +101,7 @@ extern char *logmsg_reencode(const struct commit *commit,
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
extern char *logmsg_reencode(const struct commit *commit,
const char *output_encoding);
+extern void logmsg_free(char *msg, const struct commit *commit);
extern void get_commit_format(const char *arg, struct rev_info *);
extern const char *format_subject(struct strbuf *sb, const char *msg,
const char *line_separator);
diff --git a/pretty.c b/pretty.c
index 07fc062..c675349 100644
--- a/pretty.c
+++ b/pretty.c
@@ -524,10 +524,11 @@ static char *get_header(const struct commit *commit, const char *key)
strbuf_addch(sb, '\n');
}
-static char *get_header(const struct commit *commit, const char *key)
+static char *get_header(const struct commit *commit, const char *msg,
+ const char *key)
{
int key_len = strlen(key);
- const char *line = commit->buffer;
+ const char *line = msg;
while (line) {
const char *eol = strchr(line, '\n'), *next;
@@ -588,17 +589,18 @@ char *logmsg_reencode(const struct commit *commit,
static const char *utf8 = "UTF-8";
const char *use_encoding;
char *encoding;
+ char *msg = commit->buffer;
char *out;
if (!output_encoding || !*output_encoding)
- return NULL;
- encoding = get_header(commit, "encoding");
+ return msg;
+ encoding = get_header(commit, msg, "encoding");
use_encoding = encoding ? encoding : utf8;
if (same_encoding(use_encoding, output_encoding))
if (encoding) /* we'll strip encoding header later */
out = xstrdup(commit->buffer);
else
- return NULL; /* nothing to do */
+ return msg; /* nothing to do */
else
out = reencode_string(commit->buffer,
output_encoding, use_encoding);
@@ -606,7 +608,17 @@ char *logmsg_reencode(const struct commit *commit,
out = replace_encoding_header(out, output_encoding);
free(encoding);
- return out;
+ /*
+ * If the re-encoding failed, out might be NULL here; in that
+ * case we just return the commit message verbatim.
+ */
+ return out ? out : msg;
+}
+
+void logmsg_free(char *msg, const struct commit *commit)
+{
+ if (msg != commit->buffer)
+ free(msg);
}
static int mailmap_name(const char **email, size_t *email_len,
@@ -1278,14 +1290,11 @@ void format_commit_message(const struct commit *commit,
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb->len;
context.message = logmsg_reencode(commit, output_enc);
- if (!context.message)
- context.message = commit->buffer;
strbuf_expand(sb, format, format_commit_item, &context);
rewrap_message_tail(sb, &context, 0, 0, 0);
- if (context.message != commit->buffer)
- free(context.message);
+ logmsg_free(context.message, commit);
free(context.signature.gpg_output);
free(context.signature.signer);
}
@@ -1432,7 +1441,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
{
unsigned long beginning_of_body;
int indent = 4;
- const char *msg = commit->buffer;
+ const char *msg;
char *reencoded;
const char *encoding;
int need_8bit_cte = pp->need_8bit_cte;
@@ -1443,10 +1452,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
}
encoding = get_log_output_encoding();
- reencoded = logmsg_reencode(commit, encoding);
- if (reencoded) {
- msg = reencoded;
- }
+ msg = reencoded = logmsg_reencode(commit, encoding);
if (pp->fmt == CMIT_FMT_ONELINE || pp->fmt == CMIT_FMT_EMAIL)
indent = 0;
@@ -1503,7 +1509,7 @@ void pretty_print_commit(const struct pretty_print_context *pp,
if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
strbuf_addch(sb, '\n');
- free(reencoded);
+ logmsg_free(reencoded, commit);
}
void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit,
--
1.8.0.2.16.g72e2fc9
^ permalink raw reply related
* [PATCH 3/3] logmsg_reencode: lazily load missing commit buffers
From: Jeff King @ 2013-01-26 9:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathon Mah, Jonathan Nieder, Duy Nguyen, Stefan Näwe,
Armin, git@vger.kernel.org
In-Reply-To: <20130126094026.GA9646@sigill.intra.peff.net>
Usually a commit that makes it to logmsg_reencode will have
been parsed, and the commit->buffer struct member will be
valid. However, some code paths will free commit buffers
after having used them (for example, the log traversal
machinery will do so to keep memory usage down).
Most of the time this is fine; log should only show a commit
once, and then exits. However, there are some code paths
where this does not work. At least two are known:
1. A commit may be shown as part of a regular ref, and
then it may be shown again as part of a submodule diff
(e.g., if a repo contains refs to both the superproject
and subproject).
2. A notes-cache commit may be shown during "log --all",
and then later used to access a textconv cache during a
diff.
Lazily loading in logmsg_reencode does not necessarily catch
all such cases, but it should catch most of them. Users of
the commit buffer tend to be either parsing for structure
(in which they will call parse_commit, and either we will
already have parsed, or we will load commit->buffer lazily
there), or outputting (either to the user, or fetching a
part of the commit message via format_commit_message). In
the latter case, we should always be using logmsg_reencode
anyway (and typically we do so via the pretty-print
machinery).
If there are any cases that this misses, we can fix them up
to use logmsg_reencode (or handle them on a case-by-case
basis if that is inappropriate).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/blame.c | 13 ---------
pretty.c | 57 ++++++++++++++++++++++++++++++++++------
t/t4042-diff-textconv-caching.sh | 8 ++++++
3 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 962e4e3..86100e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1424,19 +1424,6 @@ static void get_commit_info(struct commit *commit,
commit_info_init(ret);
- /*
- * We've operated without save_commit_buffer, so
- * we now need to populate them for output.
- */
- if (!commit->buffer) {
- enum object_type type;
- unsigned long size;
- commit->buffer =
- read_sha1_file(commit->object.sha1, &type, &size);
- if (!commit->buffer)
- die("Cannot read commit %s",
- sha1_to_hex(commit->object.sha1));
- }
encoding = get_log_output_encoding();
message = logmsg_reencode(commit, encoding);
get_ac_line(message, "\nauthor ",
diff --git a/pretty.c b/pretty.c
index c675349..eae57ad 100644
--- a/pretty.c
+++ b/pretty.c
@@ -592,18 +592,59 @@ char *logmsg_reencode(const struct commit *commit,
char *msg = commit->buffer;
char *out;
+ if (!msg) {
+ enum object_type type;
+ unsigned long size;
+
+ msg = read_sha1_file(commit->object.sha1, &type, &size);
+ if (!msg)
+ die("Cannot read commit object %s",
+ sha1_to_hex(commit->object.sha1));
+ if (type != OBJ_COMMIT)
+ die("Expected commit for '%s', got %s",
+ sha1_to_hex(commit->object.sha1), typename(type));
+ }
+
if (!output_encoding || !*output_encoding)
return msg;
encoding = get_header(commit, msg, "encoding");
use_encoding = encoding ? encoding : utf8;
- if (same_encoding(use_encoding, output_encoding))
- if (encoding) /* we'll strip encoding header later */
- out = xstrdup(commit->buffer);
- else
- return msg; /* nothing to do */
- else
- out = reencode_string(commit->buffer,
- output_encoding, use_encoding);
+ if (same_encoding(use_encoding, output_encoding)) {
+ /*
+ * No encoding work to be done. If we have no encoding header
+ * at all, then there's nothing to do, and we can return the
+ * message verbatim (whether newly allocated or not).
+ */
+ if (!encoding)
+ return msg;
+
+ /*
+ * Otherwise, we still want to munge the encoding header in the
+ * result, which will be done by modifying the buffer. If we
+ * are using a fresh copy, we can reuse it. But if we are using
+ * the cached copy from commit->buffer, we need to duplicate it
+ * to avoid munging commit->buffer.
+ */
+ out = msg;
+ if (out == commit->buffer)
+ out = xstrdup(out);
+ }
+ else {
+ /*
+ * There's actual encoding work to do. Do the reencoding, which
+ * still leaves the header to be replaced in the next step. At
+ * this point, we are done with msg. If we allocated a fresh
+ * copy, we can free it.
+ */
+ out = reencode_string(msg, output_encoding, use_encoding);
+ if (out && msg != commit->buffer)
+ free(msg);
+ }
+
+ /*
+ * This replacement actually consumes the buffer we hand it, so we do
+ * not have to worry about freeing the old "out" here.
+ */
if (out)
out = replace_encoding_header(out, output_encoding);
diff --git a/t/t4042-diff-textconv-caching.sh b/t/t4042-diff-textconv-caching.sh
index 91f8198..04a44d5 100755
--- a/t/t4042-diff-textconv-caching.sh
+++ b/t/t4042-diff-textconv-caching.sh
@@ -106,4 +106,12 @@ test_expect_success 'switching diff driver produces correct results' '
test_cmp expect actual
'
+# The point here is to test that we can log the notes cache and still use it to
+# produce a diff later (older versions of git would segfault on this). It's
+# much more likely to come up in the real world with "log --all -p", but using
+# --no-walk lets us reliably reproduce the order of traversal.
+test_expect_success 'log notes cache and still use cache for -p' '
+ git log --no-walk -p refs/notes/textconv/magic HEAD
+'
+
test_done
--
1.8.0.2.16.g72e2fc9
^ permalink raw reply related
* Re: git merge error question: The following untracked working tree files would be overwritten by merge
From: Carsten Fuchs @ 2013-01-26 10:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vehh95e7u.fsf@alter.siamese.dyndns.org>
Am 2013-01-25 19:07, schrieb Junio C Hamano:
> Carsten Fuchs <carsten.fuchs@cafu.de> writes:
>>> [...]
>>> $ git merge origin/master --ff-only
>>> Updating f419d57..2da6052
>>> error: The following untracked working tree files would be overwritten by merge:
>>> obsolete/e107/Readme.txt
>>> obsolete/e107/article.php
>>> obsolete/e107/backend.php
>>> [...]
>> ...
>> Compare with what Subversion did in an analogous case: When I ran "svn
>> update" and the update brought new files for which there already was
>> an untracked copy in the working directory, Subversion:
>> - started to consider the file as tracked,
>> - but left the file in the working-copy alone.
>>
>> As a result, a subsequent "svn status" might
>> a) no longer show the file at all, if the foreign copy in the
>> working directory happened to be the same as the one brought by the
>> "svn update", or
>> b) flag the file as modified, if different from the one that "svn
>> update" would have created in its place.
>
> Interesting. So before running "status", the merge is recorded (in this
> particular case you are doing ff-only so there is nothing new to
> record, but if the rest of the tree merges cleanly, the new tree
> that contains "obsolete" from the other branch you just merged will
> be the contents you record in the merge commit), and working tree is
> immediately dirty?
Yes. But I don't think it's the (svn) "status" command that does anything special.
In Git, if I understand it correctly, the final step of a "merge", "checkout", "reset",
etc. is a move of the HEAD to the resulting or specified commit. I imagine that it is
here where the diff of the dirty working tree is re-applied to the newly checked out
commit (and if this is not possible cleanly, probably [a] the whole operation must
abort, or [b] leave files in the working tree with conflict markers), and where a
decision must be made about "obstructing paths" (svn lingo): [c] abort the whole
operation, or [d] "version" them (but don't modify them in any way).
I'm not sure if Subversion does [a] and [c] ("abort") without the --force option, and
[b] and [d] with --force, or any other combination, but at least TortoiseSVN seems to
use [d] by default (which seems safe enough).
Despite a thorough search, I've not been able to find much reference about this behaviour:
http://svnbook.red-bean.com/en/1.6/svn.ref.svn.c.switch.html
http://markphip.blogspot.de/2007/01/subversion-15-tolerate-obstructions.html
However, as the blog article mentions, I too have found this treatment of obstructing
paths very natural and helpful in several occasions.
(Because without it, we must manually rename the obstructing paths, re-start the
previously aborted operation, and then take diffs or somehow else compare the renamed
obstructing and newly added paths manually, and possible merge them manually; or at
least copy the renamed edition over the newly added edition to get back into Git for the
job.)
>> So my real question is, why does Git not do something analogous?
>
> The answer to that question is "because nobody thought that doing so
> would help users very much and bothered to implement it"; it is not
> "people thought about doing so and found reasons why that would not
> help users".
Thanks, it's very good to hear this!
:-)
Best regards,
Carsten
^ permalink raw reply
* [PATCH 1/2 v2] mergetool--lib: don't call "exit" in setup_tool
From: John Keeping @ 2013-01-26 12:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Aguilar, git
In-Reply-To: <7vip6k23mk.fsf@alter.siamese.dyndns.org>
This will make it easier to use setup_tool in places where we expect
that the selected tool will not support the current mode.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Fri, Jan 25, 2013 at 04:24:03PM -0800, Junio C Hamano wrote:
> Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break
> t7610 rather badly.
Sorry about that. The 'setup_tool' function should really be called
'setup_builtin_tool' - it isn't necessary when a custom mergetool is
configured and will return 1 when called with an argument that isn't a
builtin tool from $GIT_EXEC_PATH/mergetools.
The change is the second hunk below which now wraps the call to
setup_tool in an if block as well as adding the "|| return".
git-mergetool--lib.sh | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4c1e129..8a5eaff 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -67,11 +67,11 @@ setup_tool () {
if merge_mode && ! can_merge
then
echo "error: '$tool' can not be used to resolve merges" >&2
- exit 1
+ return 1
elif diff_mode && ! can_diff
then
echo "error: '$tool' can only be used to resolve merges" >&2
- exit 1
+ return 1
fi
return 0
}
@@ -100,7 +100,10 @@ run_merge_tool () {
status=0
# Bring tool-specific functions into scope
- setup_tool "$1"
+ if test -z "$merge_tool_path"
+ then
+ setup_tool "$1" || return
+ fi
if merge_mode
then
--
1.8.1.1.367.ga9c3dd4.dirty
^ permalink raw reply related
* Re: [PATCH] mergetools: Simplify how we handle "vim" and "defaults"
From: John Keeping @ 2013-01-26 12:12 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <1359183058-51835-1-git-send-email-davvid@gmail.com>
On Fri, Jan 25, 2013 at 10:50:58PM -0800, David Aguilar wrote:
> Remove the exceptions for "vim" and "defaults" in the mergetool library
> so that every filename in mergetools/ matches 1:1 with the name of a
> valid built-in tool.
>
> Make common functions available in $MERGE_TOOLS_DIR/include/.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
>
> diff --git a/Makefile b/Makefile
> index f69979e..3bc6eb5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2724,7 +2724,7 @@ install: all
> $(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
> $(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> - $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> + cp -R mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
Shouldn't this be more like this?
$(INSTALL) -m 644 $(subst include,,$(wildcard mergetools/*)) \
'$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
$(INSTALL) -m 644 mergetools/include/* \
'$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
I'm not sure creating an 'include' directory actually buys us much over
declaring that 'vimdiff' is the real script and the others just source
it. Either way there is a single special entry in the mergetools
directory.
> ifndef NO_GETTEXT
> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
> (cd po/build/locale && $(TAR) cf - .) | \
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index aa38bd1..c866ed8 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -1,5 +1,7 @@
> #!/bin/sh
> # git-mergetool--lib is a library for common merge tool functions
> +MERGE_TOOLS_DIR="$(git --exec-path)/mergetools"
> +
> diff_mode() {
> test "$TOOL_MODE" = diff
> }
> @@ -44,25 +46,14 @@ valid_tool () {
> }
>
> setup_tool () {
> - case "$1" in
> - vim*|gvim*)
> - tool=vim
> - ;;
> - *)
> - tool="$1"
> - ;;
> - esac
> - mergetools="$(git --exec-path)/mergetools"
> + tool="$1"
Unnecessary quoting.
> - # Load the default definitions
> - . "$mergetools/defaults"
> - if ! test -f "$mergetools/$tool"
> + if ! test -f "$MERGE_TOOLS_DIR/$tool"
> then
> return 1
> fi
> -
> - # Load the redefined functions
> - . "$mergetools/$tool"
> + . "$MERGE_TOOLS_DIR/include/defaults.sh"
> + . "$MERGE_TOOLS_DIR/$tool"
>
> if merge_mode && ! can_merge
> then
> @@ -99,7 +90,7 @@ run_merge_tool () {
> base_present="$2"
> status=0
>
> - # Bring tool-specific functions into scope
> + # Bring tool specific functions into scope
This isn't related to this change (and I think tool-specific is more
correct anyway).
> setup_tool "$1"
>
> if merge_mode
> @@ -177,18 +168,17 @@ list_merge_tool_candidates () {
> show_tool_help () {
> unavailable= available= LF='
> '
> -
> - scriptlets="$(git --exec-path)"/mergetools
> - for i in "$scriptlets"/*
> + for i in "$MERGE_TOOLS_DIR"/*
> do
> - . "$scriptlets"/defaults
> - . "$i"
> -
> - tool="$(basename "$i")"
> - if test "$tool" = "defaults"
> + if test -d "$i"
> then
> continue
> - elif merge_mode && ! can_merge
> + fi
> +
> + . "$MERGE_TOOLS_DIR"/include/defaults.sh
> + . "$i"
> +
> + if merge_mode && ! can_merge
> then
> continue
> elif diff_mode && ! can_diff
> @@ -196,6 +186,7 @@ show_tool_help () {
> continue
> fi
I'd prefer to see my change to setup_tool done before this so that we
can just use:
setup_tool "$tool" 2>/dev/null || continue
for the above block. I'll send a fixed version in a couple of minutes.
> + tool=$(basename "$i")
> merge_tool_path=$(translate_merge_tool_path "$tool")
> if type "$merge_tool_path" >/dev/null 2>&1
> then
> diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
> new file mode 100644
> index 0000000..f5890b1
> --- /dev/null
> +++ b/mergetools/gvimdiff
> @@ -0,0 +1 @@
> +. "$MERGE_TOOLS_DIR/include/vim.sh"
> diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
> new file mode 100644
> index 0000000..f5890b1
> --- /dev/null
> +++ b/mergetools/gvimdiff2
> @@ -0,0 +1 @@
> +. "$MERGE_TOOLS_DIR/include/vim.sh"
> diff --git a/mergetools/defaults b/mergetools/include/defaults.sh
> similarity index 100%
> rename from mergetools/defaults
> rename to mergetools/include/defaults.sh
> diff --git a/mergetools/vim b/mergetools/include/vim.sh
> similarity index 100%
> rename from mergetools/vim
> rename to mergetools/include/vim.sh
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> new file mode 100644
> index 0000000..f5890b1
> --- /dev/null
> +++ b/mergetools/vimdiff
> @@ -0,0 +1 @@
> +. "$MERGE_TOOLS_DIR/include/vim.sh"
> diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
> new file mode 100644
> index 0000000..f5890b1
> --- /dev/null
> +++ b/mergetools/vimdiff2
> @@ -0,0 +1 @@
> +. "$MERGE_TOOLS_DIR/include/vim.sh"
^ permalink raw reply
* Re: [PATCH 1/2] git-p4.py: support Python 2.5
From: Pete Wyckoff @ 2013-01-26 12:45 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, esr, john, Brandon Casey
In-Reply-To: <1359146641-27810-2-git-send-email-drafnel@gmail.com>
drafnel@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800:
> Python 2.5 and older do not accept None as the first argument to
> translate() and complain with:
>
> TypeError: expected a character buffer object
>
> Satisfy this older python by calling maketrans() to generate an empty
> translation table and supplying that to translate().
>
> This allows git-p4 to be used with Python 2.5.
This was a lot easier than I imagined!
> def wildcard_present(path):
> - return path.translate(None, "*#@%") != path
> + from string import maketrans
> + return path.translate(maketrans("",""), "*#@%") != path
translate() was a bit too subtle already. Could you try
something like this instead?
m = re.search("[*#@%]", path)
return m is not None
I think that'll work everywhere and not force people to look
up how translate and maketrans work.
-- Pete
^ permalink raw reply
* Re: [PATCH 2/2] git-p4.py: support Python 2.4
From: Pete Wyckoff @ 2013-01-26 12:48 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, esr, john, Brandon Casey
In-Reply-To: <1359146641-27810-3-git-send-email-drafnel@gmail.com>
drafnel@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800:
> Python 2.4 lacks the following features:
>
> subprocess.check_call
> struct.pack_into
>
> Take a cue from 460d1026 and provide an implementation of the
> CalledProcessError exception. Then replace the calls to
> subproccess.check_call with calls to subprocess.call that check the return
> status and raise a CalledProcessError exception if necessary.
>
> The struct.pack_into in t/9802 can be converted into a single struct.pack
> call which is available in Python 2.4.
Excellent. Should have used struct.pack() from the get-go.
Acked-by: Pete Wyckoff <pw@padd.com>
> diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
> index 21924df..be299dc 100755
> --- a/t/t9802-git-p4-filetype.sh
> +++ b/t/t9802-git-p4-filetype.sh
> @@ -105,12 +105,13 @@ build_gendouble() {
> cat >gendouble.py <<-\EOF
> import sys
> import struct
> - import array
>
> - s = array.array("c", '\0' * 26)
> - struct.pack_into(">L", s, 0, 0x00051607) # AppleDouble
> - struct.pack_into(">L", s, 4, 0x00020000) # version 2
> - s.tofile(sys.stdout)
> + s = struct.pack(">LL18s",
> + 0x00051607, # AppleDouble
> + 0x00020000, # version 2
> + "" # pad to 26 bytes
> + )
> + sys.stdout.write(s);
> EOF
One stray semicolon.
In terms of maintenance, I'll not run tests with 2.4 or 2.5
myself, but maybe you would be willing to check an RC candidate
each release?
-- Pete
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox