* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Junio C Hamano @ 2009-01-22 0:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git
In-Reply-To: <alpine.DEB.1.00.0901212259420.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Fact is: every test script will check $GIT_VALGRIND/bin/ for
> up-to-dateness first. Before running any Git command.
Hmm, is that a good thing in general? Can't makefile rules be arranged in
such a way that one "valgrind-prep" target runs before all the potentially
parallel executions of actual tests begin?
Independent from the above, I suspect that some of the existing tests
cannot run in parallel; I haven't really looked at any of them, but a
server-ish tests to open a local port and test interaction with client
obviously need to either use different ports or serialize. Perhaps we
need a way to mark some tests that cannot be run in parallel even under
"make -j"?
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-22 0:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vr62wb28h.fsf@gitster.siamese.dyndns.org>
On Wed, Jan 21, 2009 at 04:42:22PM -0800, Junio C Hamano wrote:
> > Fact is: every test script will check $GIT_VALGRIND/bin/ for
> > up-to-dateness first. Before running any Git command.
>
> Hmm, is that a good thing in general? Can't makefile rules be arranged in
> such a way that one "valgrind-prep" target runs before all the potentially
> parallel executions of actual tests begin?
You have to choose either "everybody does this setup, whether they want
--valgrind or not" which is what my original patch did, or doing it
inside test-lib.sh. Because we don't know we want --valgrind until we
get into the individual scripts.
I suppose one could try parsing GIT_TEST_OPTS in the Makefile, but that
seems a bit hack-ish.
But I like putting it into test-lib.sh; yes, it is a little more CPU
time for each script, but it is negligible compared to running the
actual tests (especially since you only pay when running with
--valgrind, which makes the actual tests very expensive). But it is much
easier to be sure it is _correct_ when you run the test, especially if
you tend to run the test script directly.
> Independent from the above, I suspect that some of the existing tests
> cannot run in parallel; I haven't really looked at any of them, but a
> server-ish tests to open a local port and test interaction with client
> obviously need to either use different ports or serialize. Perhaps we
> need a way to mark some tests that cannot be run in parallel even under
> "make -j"?
I think the only culprits are http-push and a few SVN tests. The
http-push test starts a server on a specific port, but because it is the
only script which uses that port, it is fine. It looks like a few
different SVN tests start an httpd server (9115, 9118, and 9120), which
could potentially interact badly. I've never had a problem running with
"-j4", but I don't have svn installed, so I always end up skipping those
tests.
It looks like both the http-push and svn tests are set up to take an
arbitrary port as input. Perhaps the simplest thing would be for each of
the svn tests to pick a different port so that they can be run
simultaneously.
-Peff
^ permalink raw reply
* Re: [PATCH] Added giteditor script to show diff while editing commit message.
From: Ted Pavlic @ 2009-01-22 1:46 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <alpine.DEB.1.00.0901212343100.3586@pacific.mpi-cbg.de>
>> Thanks for your comments. I've responded below. I just want to
>> top-respond to your comment that the fundamental problem is that the
>> diff is in a separate file. In fact, this is the point of the script. I
>> want to be able to scroll through the diff output independent of the
>> commit message.
> Once again, note that e.g. vi will not cope with the way you try to
> achieve that.
It does for me just fine. In vi, I hit ^W^W and move from commit message
to diff and back. What's the problem with that? In gvim I'm able to
click back and forth.
I have been using this method for a long time with hg, and now I've been
using it with git. This isn't theory --- it's been working in practice
for me. Am I missing something?
> Why not .git/? That would be the _natural_ place to put it.
Why doesn't stg do that? I figured stg would be a well-established
program to pattern behavior off of.
I'll rev-parse the git dir and place the file there.
>>> vi users will hate you, as you do not give them a chance to edit the
>>> message after having seen the diff.
>> I don't see what you mean. I am a vi user (exclusively), and this script
>> works very well for me.
> I cannot go back to the commit message when I said ":n" to get to the
> diff.
vi opens for me and I see two windows. The top window shows the commit
message and the bottom window shows the diff.
I hit ^W^W (or ^W<Down>) and I find myself scrolling around in the diff.
I hit ^W^W again (or ^W<Up>) and I find myself scrolling around in the
commit.
Similarly, gvim lets me mouse around both --- clicking from window to
window.
If you must use ":n", I don't know why you can't use ":prev" to go back. <?>
>> Is there no value in having the diff in a separate file?
> In my case, no, for 2 reasons:
> - I can always open a new shell (in ssh connections, I use screen) to get
> the diff, and even better: I can restrict it to certain files, and I can
> use the nice bookmarks "less" provides; dunno if vi would have them.
vi does.
> - My preference is definitely to look at the diff before committing, to be
> certain that I did not fsck up. And nothing would annoy me more than to
> be in the middle of editing a commit message while I am looking at the
> diff and telling myself "that is a stupid mistake, let's fix it" knowing
> that the commit will not pick up the fix.
When giving a detailed message bulletting out everything that goes into
a commit, sometimes it's nice to have a very nearby look at the diff.
> So seeing the diff while composing the commit message is definitely too
> late for me.
Nevertheless, the secondary purpose of the contributed script is to show
how GIT_EDITOR can be used to wrap around other editors. (that's the
purpose of Mercurial's distributed "hgeditor" script as well)
--Ted
--
Ted Pavlic <ted@tedpavlic.com>
Please visit my ALS association page:
http://web.alsa.org/goto/tedpavlic
My family appreciates your support in the fight to defeat ALS.
^ permalink raw reply
* Re: [PATCH] contrib: A script to show diff in new window while editing commit message.
From: Ted Pavlic @ 2009-01-22 3:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy6x4b48e.fsf@gitster.siamese.dyndns.org>
> * Some people would like two-buffer operation and they may use this
> script as their core.editor.
That's what I do, and evidently that's what enough Mercurial users do to
warrant them putting "hgeditor" in their main distro (not even in the
"contrib" directory). I just figured there must be at least a few other
people out there like me, but maybe I'm wrong. <?>
> * Other people (including me) would find it very natural to use "\C-x 2"
> if they need to look at two places of the same buffer, because that is
> what they are used to do when editing a long file every day. They just
> use "commit -v" without bothering with this script.
As I mention in the script, splitting windows (which is easy to do in
Vim) is another very natural way to go. That being said, not all editors
have terrific split window support. Plus, if you're going to do it
often, it makes sense to wire up your editor to do it for you each time.
> * Yet other people (like Dscho) would find it too late to have a chance
> for final review when writing a commit log message anyway, and won't
> use either.
I'm not using the diff as a code review. I'm using it to help me make
sure my commit message is complete.
> Having said that, if the lack of "final chance to review the diff" in some
> StGIT subcommand is the real problem you are trying to solve, I think it
...
Having the "stg edit" support is just a bonus (for me). The main point
was showing how to use GIT_EDITOR to bring up a split window. As with
Mercurial's hgeditor, part of the point of the script is to demonstrate
how to customize the commit process.
Thanks --
--Ted
--
Ted Pavlic <ted@tedpavlic.com>
Please visit my ALS association page:
http://web.alsa.org/goto/tedpavlic
My family appreciates your support in the fight to defeat ALS.
^ permalink raw reply
* [PATCH] contrib: A script to show diff in new window while editing commit message.
From: Ted Pavlic @ 2009-01-22 3:50 UTC (permalink / raw)
To: gitster; +Cc: git, Ted Pavlic
In-Reply-To: <7vy6x4b48e.fsf@gitster.siamese.dyndns.org>
This new script (contrib/giteditor/giteditor) is an example GIT_EDITOR
that causes the editor to open the commit message as well as a "git diff
--cached" in a separate window. This behavior differs from "git commit
-v" in that the diff can be browsed independently of the commit message
without having to invoke a split window view in an editor.
This script also detects when "stg edit" is being called and uses "stg
show" instead. Hence, it implements a kind of "stg edit -v".
This script is highly influenced by the "hgeditor" script distributed
with the Mercurial SCM.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
---
This new commit responds to some of the issues brought up by Junio C
Hemano (and Johannes Schindelin). In particular, it removes the
paragraph from the commit message discussing how it could be "improved."
Also, this new version uses a "DIFFPIPE" to strip the old commit message
from the top of the "stg show" output so that the "stg edit" behavior
matches the "git commit" behavior.
Finally, this version adds a comment giving an idea for personalizing by
adding the temporary directory creation back in (as a way to prevent
editor backup files from piling up inside .git).
contrib/giteditor/giteditor | 86 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 86 insertions(+), 0 deletions(-)
create mode 100755 contrib/giteditor/giteditor
diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
new file mode 100755
index 0000000..5369732
--- /dev/null
+++ b/contrib/giteditor/giteditor
@@ -0,0 +1,86 @@
+#!/bin/sh
+#
+# Set GIT_EDITOR (or core.editor) to this script to see a diff alongside
+# commit message. This script differs from "git commit -v" in that the
+# diff shows up in a separate buffer. Additionally, this script works
+# with "stg edit" as well.
+#
+# Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
+# Highly influenced by hgeditor script distributed with Mercurial SCM.
+# Distributed under the GNU General Public License, version 2.0.
+#
+# Possible personalizations:
+#
+# * If your GIT_DIR gets polluted with backup files created by your
+# editor when COMMIT_EDITMSG is saved, then have this script copy
+# COMMIT_EDITMSG (i.e., $1) to a temporary directory and then back to
+# COMMIT_EDITMSG when done. When the script cleans up after itself, it
+# can delete the temporary directory and any leftover backup files.
+# Note that the vim setting 'nobackup' disables saving backup files,
+# and this setting can be set automatically on gitcommit-type files
+# and files matching .stgit-*.txt with an appropriate ftdetect entry.
+
+# Find git
+test -z "${GIT}" && GIT="git"
+
+# Find stg
+test -z "${STG}" && STG="stg"
+
+# Find the nearest git-dir
+GITDIR=$(git rev-parse --git-dir) || exit
+
+# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
+EDITOR="${GIT_EDITOR_EDITOR-${VISUAL-${EDITOR-vi}}}"
+
+# If we recognize a popular editor, add necessary flags (e.g., to
+# prevent forking)
+case "${EDITOR}" in
+ emacs)
+ EDITOR="${EDITOR} -nw"
+ ;;
+ mvim|gvim|vim|vi)
+ EDITOR="${EDITOR} -f -o"
+ ;;
+esac
+
+# Remove temporary files even if we get interrupted
+DIFFOUTPUT="${GITDIR}/giteditor.${RANDOM}.${RANDOM}.${RANDOM}.$$.diff"
+cleanup_exit() {
+ rm -f "${DIFFOUTPUT}"
+}
+trap "cleanup_exit" 0 # normal exit
+trap "exit 255" 1 2 3 6 15 # HUP INT QUIT ABRT TERM
+
+# For git, COMMITMSG=COMMIT_EDITMSG
+# For stg, COMMITMSG=.stgit-edit.txt
+# etc.
+COMMITMSG=$(basename "$1")
+DIFFPIPE="cat"
+case "${COMMITMSG}" in
+ .stgit-edit.txt) # From "stg edit"
+ DIFFCMD="${STG}"
+ DIFFARGS="show"
+ DIFFPIPE="tail +$(wc -l "$1"|awk '{print $1+3}')"
+ ;;
+ *) # Fall through to "git commit" case
+ DIFFCMD="${GIT}"
+ DIFFARGS="diff --cached"
+ # To focus on files that changed, use:
+ #DIFFARGS="diff --cached --diff-filter=M"
+ ;;
+esac
+
+# Do the diff and save the result in DIFFOUTPUT
+"${DIFFCMD}" ${DIFFARGS} | ${DIFFPIPE} > ${DIFFOUTPUT}
+
+# If DIFFOUTPUT is nonempty, open it alongside commit message
+if test -s "${DIFFOUTPUT}"; then
+ # Diff is non-empty, so edit msg and diff
+ ${EDITOR} "$1" "${DIFFOUTPUT}" || exit
+else
+ # Empty diff. Only edit msg
+ ${EDITOR} "$1" || exit
+fi
+
+# (recall that DIFFOUTPUT file gets cleaned up by trap above)
+exit
--
1.6.1.213.g28da8
^ permalink raw reply related
* What's cooking in git.git (Jan 2009, #05; Wed, 21)
From: Junio C Hamano @ 2009-01-22 3:55 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with '-' are
only in 'pu' while commits prefixed with '+' are in 'next'. The ones
marked with '.' do not appear in any of the branches, but I am still
holding onto them.
The topics list the commits in reverse chronological order. The topics
meant to be merged to the maintenance series have "maint-" in their names.
----------------------------------------------------------------
[New Topics]
* js/valgrind (Wed Jan 21 02:36:40 2009 +0100) 2 commits
- valgrind: ignore ldso errors
- Add valgrind support in test scripts
Dscho seems to have some updates out of discussion with Peff.
----------------------------------------------------------------
[Stalled and may need help and prodding to go forward]
* jc/blame (Wed Jun 4 22:58:40 2008 -0700) 2 commits
+ blame: show "previous" information in --porcelain/--incremental
format
+ git-blame: refactor code to emit "porcelain format" output
This gives Porcelains (like gitweb) the information on the commit _before_
the one that the final blame is laid on, which should save them one
rev-parse to dig further. The line number in the "previous" information
may need refining, and sanity checking code for reference counting may
need to be resurrected before this can move forward.
* db/foreign-scm (Sun Jan 11 15:12:10 2009 -0500) 3 commits
- Support fetching from foreign VCSes
- Add specification of git-vcs helpers
- Add "vcs" config option in remotes
The "spec" did not seem quite well cooked yet, but in the longer term I
think something like this to allow interoperating with other SCMs as if
the other end is a native git repository is a very worthy goal.
----------------------------------------------------------------
[Actively cooking]
* sp/runtime-prefix (Sun Jan 18 13:00:15 2009 +0100) 7 commits
- Windows: Revert to default paths and convert them by
RUNTIME_PREFIX
- Compute prefix at runtime if RUNTIME_PREFIX is set
- Modify setup_path() to only add git_exec_path() to PATH
- Add calls to git_extract_argv0_path() in programs that call
git_config_*
- git_extract_argv0_path(): Move check for valid argv0 from caller
to callee
- Refactor git_set_argv0_path() to git_extract_argv0_path()
- Move computation of absolute paths from Makefile to runtime (in
preparation for RUNTIME_PREFIX)
We should move this to 'next' soon with J6t's blessing.
* lh/submodule-tree-traversal (Mon Jan 12 00:45:55 2009 +0100) 3 commits
+ builtin-ls-tree: enable traversal of submodules
+ archive.c: enable traversal of submodules
+ tree.c: add support for traversal of submodules
I think choosing the submodules to descend into by seeing if the commit
happens to be available is a horribly broken semantics; it needs to be
fixed before this can move to 'master'.
* jk/signal-cleanup (Sun Jan 11 06:36:49 2009 -0500) 3 commits
- pager: do wait_for_pager on signal death
- refactor signal handling for cleanup functions
- chain kill signals for cleanup functions
I think this can move to 'next', as Peff and J6t agreed on how to fix
things up as needed for Windows.
* ks/maint-mailinfo-folded (Tue Jan 13 01:21:04 2009 +0300) 5 commits
- mailinfo: tests for RFC2047 examples
- mailinfo: add explicit test for mails like '<a.u.thor@example.com>
(A U Thor)'
- mailinfo: more smarter removal of rfc822 comments from 'From'
+ mailinfo: 'From:' header should be unfold as well
+ mailinfo: correctly handle multiline 'Subject:' header
As far as I can see, the only remaining thing is a minor fix-up in the
"comment removal" one before we can move this fully to 'next'.
* js/notes (Tue Jan 13 20:57:16 2009 +0100) 6 commits
+ git-notes: fix printing of multi-line notes
+ notes: fix core.notesRef documentation
+ Add an expensive test for git-notes
+ Speed up git notes lookup
+ Add a script to edit/inspect notes
+ Introduce commit notes
It would be nice to hear a real world success story using the notes
mechanism before casting this design in stone.
* sc/gitweb-category (Fri Dec 12 00:45:12 2008 +0100) 3 commits
- gitweb: Optional grouping of projects by category
- gitweb: Split git_project_list_body in two functions
- gitweb: Modularized git_get_project_description to be more generic
Design discussion between Jakub and Sebastien continues.
----------------------------------------------------------------
[Graduated to "master"]
* jk/color-parse (Sat Jan 17 10:38:46 2009 -0500) 2 commits
+ expand --pretty=format color options
+ color: make it easier for non-config to parse color specs
* sb/hook-cleanup (Sat Jan 17 04:02:55 2009 +0100) 5 commits
+ run_hook(): allow more than 9 hook arguments
+ run_hook(): check the executability of the hook before filling
argv
+ api-run-command.txt: talk about run_hook()
+ Move run_hook() from builtin-commit.c into run-command.c (libgit)
+ checkout: don't crash on file checkout before running post-
checkout hook
* rs/ctype (Sat Jan 17 16:50:37 2009 +0100) 4 commits
+ Add is_regex_special()
+ Change NUL char handling of isspecial()
+ Reformat ctype.c
+ Add ctype test
* jf/am-failure-report (Sun Jan 18 19:34:31 2009 -0800) 2 commits
+ git-am: re-fix the diag message printing
+ git-am: Make it easier to see which patch failed
* sg/maint-gitdir-in-subdir (Fri Jan 16 16:37:33 2009 +0100) 1 commit
+ Fix gitdir detection when in subdir of gitdir
This has my "don't do the fullpath if you are directly inside .git"
squashed in, so it should be much safer.
* am/maint-push-doc (Sun Jan 18 15:36:58 2009 +0100) 4 commits
+ Documentation: avoid using undefined parameters
+ Documentation: mention branches rather than heads
+ Documentation: remove a redundant elaboration
+ Documentation: git push repository can also be a remote
* lt/maint-wrap-zlib (Wed Jan 7 19:54:47 2009 -0800) 1 commit
+ Wrap inflate and other zlib routines for better error reporting
Needs the "free our memory upon seeing Z_MEM_ERROR and try again" bits
extracted from Shawn's patch on top of this one.
* kb/am-directory (Wed Jan 14 16:29:59 2009 -0800) 2 commits
+ git-am: fix shell quoting
+ git-am: add --directory=<dir> option
This is "third-time-lucky, perhaps?" resurrection. I do not think I'd be
using this very often, but it originated from a real user request.
* jc/maint-format-patch-o-relative (Mon Jan 12 15:18:02 2009 -0800) 1 commit
+ Teach format-patch to handle output directory relative to cwd
----------------------------------------------------------------
[Will merge to "master" soon]
* kb/lstat-cache (Sun Jan 18 16:14:54 2009 +0100) 5 commits
+ lstat_cache(): introduce clear_lstat_cache() function
+ lstat_cache(): introduce invalidate_lstat_cache() function
+ lstat_cache(): introduce has_dirs_only_path() function
+ lstat_cache(): introduce has_symlink_or_noent_leading_path()
function
+ lstat_cache(): more cache effective symlink/directory detection
* tr/previous-branch (Wed Jan 21 00:37:38 2009 -0800) 10 commits
+ Simplify parsing branch switching events in reflog
+ Introduce for_each_recent_reflog_ent().
+ interpret_nth_last_branch(): plug small memleak
+ Fix reflog parsing for a malformed branch switching entry
+ Fix parsing of @{-1}@{1}
+ interpret_nth_last_branch(): avoid traversing the reflog twice
+ checkout: implement "-" abbreviation, add docs and tests
+ sha1_name: support @{-N} syntax in get_sha1()
+ sha1_name: tweak @{-N} lookup
+ checkout: implement "@{-N}" shortcut name for N-th last branch
* js/maint-all-implies-HEAD (Sat Jan 17 22:27:08 2009 -0800) 2 commits
+ bundle: allow the same ref to be given more than once
+ revision walker: include a detached HEAD in --all
* mh/unify-color (Sun Jan 18 21:39:12 2009 +0100) 2 commits
+ move the color variables to color.c
+ handle color.ui at a central place
* cb/add-pathspec (Wed Jan 14 15:54:35 2009 +0100) 2 commits
+ remove pathspec_match, use match_pathspec instead
+ clean up pathspec matching
* js/diff-color-words (Tue Jan 20 21:46:57 2009 -0600) 8 commits
+ color-words: Support diff.wordregex config option
+ color-words: make regex configurable via attributes
+ color-words: expand docs with precise semantics
+ color-words: enable REG_NEWLINE to help user
+ color-words: take an optional regular expression describing words
+ color-words: change algorithm to allow for 0-character word
boundaries
+ color-words: refactor word splitting and use ALLOC_GROW()
+ Add color_fwrite_lines(), a function coloring each line
individually
* js/patience-diff (Thu Jan 1 17:39:37 2009 +0100) 3 commits
+ bash completions: Add the --patience option
+ Introduce the diff option '--patience'
+ Implement the patience diff algorithm
----------------------------------------------------------------
[On Hold]
* jk/renamelimit (Sat May 3 13:58:42 2008 -0700) 1 commit
. diff: enable "too large a rename" warning when -M/-C is explicitly
asked for
* jc/stripspace (Sun Mar 9 00:30:35 2008 -0800) 6 commits
. git-am --forge: add Signed-off-by: line for the author
. git-am: clean-up Signed-off-by: lines
. stripspace: add --log-clean option to clean up signed-off-by:
lines
. stripspace: use parse_options()
. Add "git am -s" test
. git-am: refactor code to add signed-off-by line for the committer
* jc/post-simplify (Fri Aug 15 01:34:51 2008 -0700) 2 commits
. revision --simplify-merges: incremental simplification
. revision --simplify-merges: prepare for incremental simplification
* wp/add-patch-find (Thu Nov 27 04:08:03 2008 +0000) 3 commits
. In add --patch, Handle K,k,J,j slightly more gracefully.
. Add / command in add --patch
. git-add -i/-p: Change prompt separater from slash to comma
* jc/grafts (Wed Jul 2 17:14:12 2008 -0700) 1 commit
. [BROKEN wrt shallow clones] Ignore graft during object transfer
* jc/replace (Fri Oct 31 09:21:39 2008 -0700) 1 commit
. WIP
^ permalink raw reply
* Re: CR codes from git commands
From: Brent Goodrick @ 2009-01-22 4:00 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901211636340.3586@pacific.mpi-cbg.de>
On Wed, Jan 21, 2009 at 7:38 AM, Johannes Schindelin
> In my test I performed one minute ago, "git pull | cat" did not show any
> CR. Maybe it is the "git" instead of "get" :-)
<snip>
> > Thanks. The fix should be to arrange it so that I can set something so
> > that a bare call such as (but just "git pull"):
> >
> > git pull
> >
> > will emit no CR codes at all, ever, regardless of if there is a tty.
> > Even if it is an env var, but a config setting would be ok too.
>
> I would actually think that it should not be an env var or config setting
> if piping it to "cat" does what you want: if the output is a tty, I think
> it is safe to assume that you want to see the progress, and if you don't,
> "| cat" is not an unreasonable thing to ask for.
You might not be able to see those CR codes from your terminal,
because ... well ... its a terminal which will process them. And if
you can't reproduce it in your environment, you'll have to duplicate
my environment, and at that is well beyond what I would ask anyone to
do. Thanks for your effort in looking into it. If I get annoyed
enough with it, I'll debug the code myself and propose a patch (but
don't hold your breath because I'm still learning this complex tool).
Thanks!
Brent
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #05; Wed, 21)
From: Jeff King @ 2009-01-22 4:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab9kataf.fsf@gitster.siamese.dyndns.org>
On Wed, Jan 21, 2009 at 07:55:36PM -0800, Junio C Hamano wrote:
> * jk/signal-cleanup (Sun Jan 11 06:36:49 2009 -0500) 3 commits
> - pager: do wait_for_pager on signal death
> - refactor signal handling for cleanup functions
> - chain kill signals for cleanup functions
>
> I think this can move to 'next', as Peff and J6t agreed on how to fix
> things up as needed for Windows.
Please wait for my re-roll, which I'll send in a few minutes.
-Peff
^ permalink raw reply
* Re: CR codes from git commands
From: Daniel Barkalow @ 2009-01-22 4:47 UTC (permalink / raw)
To: Brent Goodrick; +Cc: git
In-Reply-To: <18805.64312.289059.660023@hungover.brentg.com>
On Tue, 20 Jan 2009, Brent Goodrick wrote:
>
> Hi,
>
> I am considering converting from CVS over to using git. I'm currently
> using git version 1.5.6.5 on Debian Linux "testing". One of the first
> things I ran into was having to set PAGER to "cat" to avoid the
> problems when running git from anything other than a terminal. The
> second thing is that "git pull" (and possibly other commands) are
> emitting ^M (octal 013) codes on output, possibly caused by the same
> assumption as causes the problem that is fixed by setting PAGER to
> "cat". This is not a big deal on small repos, but on larger ones I
> actually do want to see status line output (or be given some option to
> see them), so that I can then run "tail -1lf" on the log file that is
> written during a long "git pull" operation.
It's kind of unclear what you're trying to do here. I'm guessing that
you're trying to run git with stdio directed to a /dev/tty device, where
isatty() is true, but which doesn't interpret ASCII control characters as
such. We're not detecting that you can't use a pager on this, and so you
have to use PAGER=cat (which might not be a bad idea for things like
"man", either). With some clues about the environment, we should be able
to do something about this.
You're also trying to send the progress output to a log file that you can
look at the end of (presumably in a more capable terminal). It should be
possible (with an option) to get git to output progress info to a non-tty,
and not use the CRs if the output isn't a tty.
Or do you want to use a tty that can't handle CRs, and get newlines
instead of CRs? (If I'd git on the first computer I used, it would have
printed the progress bar over and over in place and probably torn a hole
in the paper, but I haven't used that one in over 20 years.)
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Johannes Schindelin @ 2009-01-22 5:02 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090122005901.GA10826@sigill.intra.peff.net>
Hi,
On Wed, 21 Jan 2009, Jeff King wrote:
> On Wed, Jan 21, 2009 at 04:42:22PM -0800, Junio C Hamano wrote:
>
> > Independent from the above, I suspect that some of the existing tests
> > cannot run in parallel; I haven't really looked at any of them, but a
> > server-ish tests to open a local port and test interaction with client
> > obviously need to either use different ports or serialize. Perhaps we
> > need a way to mark some tests that cannot be run in parallel even
> > under "make -j"?
>
> I think the only culprits are http-push and a few SVN tests. The
> http-push test starts a server on a specific port, but because it is the
> only script which uses that port, it is fine. It looks like a few
> different SVN tests start an httpd server (9115, 9118, and 9120), which
> could potentially interact badly. I've never had a problem running with
> "-j4", but I don't have svn installed, so I always end up skipping those
> tests.
>
> It looks like both the http-push and svn tests are set up to take an
> arbitrary port as input. Perhaps the simplest thing would be for each of
> the svn tests to pick a different port so that they can be run
> simultaneously.
I _suspect_ that the svn tests already use different ports (or can work
with the same httpd), as I have subversion installed and run with -j50
regularly.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] mergetool merge/skip/abort
From: Caleb Cushing @ 2009-01-22 5:07 UTC (permalink / raw)
To: markus.heidelberg; +Cc: git
In-Reply-To: <200901211949.53432.markus.heidelberg@web.de>
> This looks to me like no merge will happen if --no-prompt/-y or
> mergetool.prompt is set to false. Have you tested with this option or
> have I misread?
sorry haven't tested as I don't use that. will test in the morning...
if it doesn't work will try to get it working.
> Also, I think you've lost some tabs. Mergetool does have some
> inconsistent tabbing but they way I've been aiming towards (which
> matches most, but not all of git-mergetool.sh) is to use tabs == 8
> spaces for indents but to indent each level by 4 spaces. e.g. three
> levels of indent is one tab plus four spaces.
thanks wasn't sure on the indentation, I set tabstop to 4 spaces in
vim so my tabs look like your spaces. I'll correct in the next case.
> It might be quite nice to offer the option of directly using an 'ours'
> or 'theirs' as an alternative to skip for binary files. A bit like
> symlinks are handled in mergetool.
I could look into it... at the same time I don't have a good test case
so I'd rather leave it to someone else.
> I'd like to keep (additionally) the behaviour, that the merge starts
> with just pressing <Enter>. Because what you mostly want to do, when
> using git-mergetool, is actually merging.
I'd thought of that... and I'll see what I can do, although to me it
doesn't matter much.
> This doesn't seem right.
erm.. yeah... I'll fix it
--
Caleb Cushing
http://xenoterracide.blogspot.com
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #05; Wed, 21)
From: Johannes Schindelin @ 2009-01-22 5:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab9kataf.fsf@gitster.siamese.dyndns.org>
Hi,
On Wed, 21 Jan 2009, Junio C Hamano wrote:
> * js/notes (Tue Jan 13 20:57:16 2009 +0100) 6 commits
> + git-notes: fix printing of multi-line notes
> + notes: fix core.notesRef documentation
> + Add an expensive test for git-notes
> + Speed up git notes lookup
> + Add a script to edit/inspect notes
> + Introduce commit notes
>
> It would be nice to hear a real world success story using the notes
> mechanism before casting this design in stone.
I'd like to have some profiling done before that. For example, I am still
a bit unsure how the things would perform with a 50-deep delta chain for
a notes tree having 50,000+ notes in it (which I think will not be all
that unreasonable for a medium-sized project that stores bug-tracking
information in the notes).
I have a gut feeling that the performance dip I saw is a direct result of
doing away with the fan-out "subdirectories": remember, originally, I had
a tree structure much like the loose objects in .git/objects/??/, while
Peff convinced me that a flat tree object should be enough.
I could be wrong on that, though.
> * js/patience-diff (Thu Jan 1 17:39:37 2009 +0100) 3 commits
> + bash completions: Add the --patience option
> + Introduce the diff option '--patience'
> + Implement the patience diff algorithm
There is this one issue that my patience's output differs from bzr's.
Since the patience diff algorithm is so lousily documented, I do not know
if it is due to my misunderstanding the algorithm, or due to bzr doing
something clever in addition.
I'd be thankful if somebody could clarify that issue.
Ciao,
Dscho
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #05; Wed, 21)
From: Boyd Stephen Smith Jr. @ 2009-01-22 5:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab9kataf.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 2008 bytes --]
On Wednesday 21 January 2009, Junio C Hamano <gitster@pobox.com> wrote
about 'What's cooking in git.git (Jan 2009, #05; Wed, 21)':
>* js/notes (Tue Jan 13 20:57:16 2009 +0100) 6 commits
> + git-notes: fix printing of multi-line notes
> + notes: fix core.notesRef documentation
> + Add an expensive test for git-notes
> + Speed up git notes lookup
> + Add a script to edit/inspect notes
> + Introduce commit notes
>
>It would be nice to hear a real world success story using the notes
>mechanism before casting this design in stone.
I'll see if I can't try to put this through some paces over the week.
Also, I'd like to see some support for notes in push/fetch, but it could
certainly be added afterwards.
>* js/diff-color-words (Tue Jan 20 21:46:57 2009 -0600) 8 commits
> + color-words: Support diff.wordregex config option
> + color-words: make regex configurable via attributes
> + color-words: expand docs with precise semantics
> + color-words: enable REG_NEWLINE to help user
> + color-words: take an optional regular expression describing words
> + color-words: change algorithm to allow for 0-character word
> boundaries
> + color-words: refactor word splitting and use ALLOC_GROW()
> + Add color_fwrite_lines(), a function coloring each line
> individually
I think my patch in
http://thread.gmane.org/gmane.comp.version-control.git/106567 should be
applied to the top of this. It respells "wordregex" to match existing
uses throughout the repo. Dscho had issues with one hunk, but I think I
addressed them in my follow-up. It looks like 98a4d87b (color-words:
Support diff.wordregex config option) still has the internally-consistent
runtogether spelling that doesn't match other configuration variables etc.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-22 5:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901220601500.3586@pacific.mpi-cbg.de>
On Thu, Jan 22, 2009 at 06:02:51AM +0100, Johannes Schindelin wrote:
> I _suspect_ that the svn tests already use different ports (or can work
> with the same httpd), as I have subversion installed and run with -j50
> regularly.
I think you're just not running them; it looks like they bail if
SVN_HTTPD_PORT isn't set by the user.
-Peff
^ permalink raw reply
* [PATCH v2 1/5] Windows: Fix signal numbers
From: Jeff King @ 2009-01-22 5:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <20090122042643.GB31427@coredump.intra.peff.net>
From: Johannes Sixt <j6t@kdbg.org>
We had defined some SIG_FOO macros that appear in the code, but that are
not supported on Windows, in order to make the code compile. But a
subsequent change will assert that a signal number is non-zero. We now
use the signal numbers that are commonly used on POSIX systems.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jeff King <peff@peff.net>
---
This is necessary to avoid violating sigchain assertions in the next
patch.
compat/mingw.h | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 4f275cb..a255898 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -21,12 +21,12 @@ typedef int pid_t;
#define WEXITSTATUS(x) ((x) & 0xff)
#define WIFSIGNALED(x) ((unsigned)(x) > 259)
-#define SIGKILL 0
-#define SIGCHLD 0
-#define SIGPIPE 0
-#define SIGHUP 0
-#define SIGQUIT 0
-#define SIGALRM 100
+#define SIGHUP 1
+#define SIGQUIT 3
+#define SIGKILL 9
+#define SIGPIPE 13
+#define SIGALRM 14
+#define SIGCHLD 17
#define F_GETFD 1
#define F_SETFD 2
--
1.6.1.403.g6c435
^ permalink raw reply related
* [PATCH v2 2/5] diff: refactor tempfile cleanup handling
From: Jeff King @ 2009-01-22 5:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <20090122042643.GB31427@coredump.intra.peff.net>
There are two pieces of code that create tempfiles for diff:
run_external_diff and run_textconv. The former cleans up its
tempfiles in the face of premature death (i.e., by die() or
by signal), but the latter does not. After this patch, they
will both use the same cleanup routines.
To make clear what the change is, let me first explain what
happens now:
- run_external_diff uses a static global array of 2
diff_tempfile structs (since it knows it will always
need exactly 2 tempfiles). It calls prepare_temp_file
(which doesn't know anything about the global array) on
each of the structs, creating the tempfiles that need to
be cleaned up. It then registers atexit and signal
handlers to look through the global array and remove the
tempfiles. If it succeeds, it calls the handler manually
(which marks the tempfile structs as unused).
- textconv has its own tempfile struct, which it allocates
using prepare_temp_file and cleans up manually. No
signal or atexit handlers.
The new code moves the installation of cleanup handlers into
the prepare_temp_file function. Which means that that
function now has to understand that there is static tempfile
storage. So what happens now is:
- run_external_diff calls prepare_temp_file
- prepare_temp_file calls claim_diff_tempfile, which
allocates an unused slot from our global array
- prepare_temp_file installs (if they have not already
been installed) atexit and signal handlers for cleanup
- prepare_temp_file sets up the tempfile as usual
- prepare_temp_file returns a pointer to the allocated
tempfile
The advantage being that run_external_diff no longer has to
care about setting up cleanup handlers. Now by virtue of
calling prepare_temp_file, run_textconv gets the same
benefit, as will any future users of prepare_temp_file.
There are also a few side benefits to the specific
implementation:
- we now install cleanup handlers _before_ allocating the
tempfile, closing a race which could leave temp cruft
- when allocating a slot in the global array, we will now
detect a situation where the old slots were not properly
vacated (i.e., somebody forgot to call remove upon
leaving the function). In the old code, such a situation
would silently overwrite the tempfile names, meaning we
would forget to clean them up. The new code dies with a
bug warning.
- we make sure only to install the signal handler once.
This isn't a big deal, since we are just overwriting the
old handler, but will become an issue when a later patch
converts the code to use sigchain
Signed-off-by: Jeff King <peff@peff.net>
---
This patch is new since v1. I started with just looking at whether it
was safe to move the signal() call, but there were really several things
to be cleaned up, and it just made sense to do them all together.
diff.c | 107 +++++++++++++++++++++++++++++++++-------------------------------
1 files changed, 55 insertions(+), 52 deletions(-)
diff --git a/diff.c b/diff.c
index 0731313..ae6d552 100644
--- a/diff.c
+++ b/diff.c
@@ -167,6 +167,33 @@ static struct diff_tempfile {
char tmp_path[PATH_MAX];
} diff_temp[2];
+static struct diff_tempfile *claim_diff_tempfile(void) {
+ int i;
+ for (i = 0; i < ARRAY_SIZE(diff_temp); i++)
+ if (!diff_temp[i].name)
+ return diff_temp + i;
+ die("BUG: diff is failing to clean up its tempfiles");
+}
+
+static int remove_tempfile_installed;
+
+static void remove_tempfile(void)
+{
+ int i;
+ for (i = 0; i < ARRAY_SIZE(diff_temp); i++)
+ if (diff_temp[i].name == diff_temp[i].tmp_path) {
+ unlink(diff_temp[i].name);
+ diff_temp[i].name = NULL;
+ }
+}
+
+static void remove_tempfile_on_signal(int signo)
+{
+ remove_tempfile();
+ signal(SIGINT, SIG_DFL);
+ raise(signo);
+}
+
static int count_lines(const char *data, int size)
{
int count, ch, completely_empty = 1, nl_just_seen = 0;
@@ -1859,10 +1886,11 @@ static void prep_temp_blob(struct diff_tempfile *temp,
sprintf(temp->mode, "%06o", mode);
}
-static void prepare_temp_file(const char *name,
- struct diff_tempfile *temp,
- struct diff_filespec *one)
+static struct diff_tempfile *prepare_temp_file(const char *name,
+ struct diff_filespec *one)
{
+ struct diff_tempfile *temp = claim_diff_tempfile();
+
if (!DIFF_FILE_VALID(one)) {
not_a_valid_file:
/* A '-' entry produces this for file-2, and
@@ -1871,7 +1899,13 @@ static void prepare_temp_file(const char *name,
temp->name = "/dev/null";
strcpy(temp->hex, ".");
strcpy(temp->mode, ".");
- return;
+ return temp;
+ }
+
+ if (!remove_tempfile_installed) {
+ atexit(remove_tempfile);
+ signal(SIGINT, remove_tempfile_on_signal);
+ remove_tempfile_installed = 1;
}
if (!one->sha1_valid ||
@@ -1911,7 +1945,7 @@ static void prepare_temp_file(const char *name,
*/
sprintf(temp->mode, "%06o", one->mode);
}
- return;
+ return temp;
}
else {
if (diff_populate_filespec(one, 0))
@@ -1919,24 +1953,7 @@ static void prepare_temp_file(const char *name,
prep_temp_blob(temp, one->data, one->size,
one->sha1, one->mode);
}
-}
-
-static void remove_tempfile(void)
-{
- int i;
-
- for (i = 0; i < 2; i++)
- if (diff_temp[i].name == diff_temp[i].tmp_path) {
- unlink(diff_temp[i].name);
- diff_temp[i].name = NULL;
- }
-}
-
-static void remove_tempfile_on_signal(int signo)
-{
- remove_tempfile();
- signal(SIGINT, SIG_DFL);
- raise(signo);
+ return temp;
}
/* An external diff command takes:
@@ -1954,34 +1971,22 @@ static void run_external_diff(const char *pgm,
int complete_rewrite)
{
const char *spawn_arg[10];
- struct diff_tempfile *temp = diff_temp;
int retval;
- static int atexit_asked = 0;
- const char *othername;
const char **arg = &spawn_arg[0];
- othername = (other? other : name);
- if (one && two) {
- prepare_temp_file(name, &temp[0], one);
- prepare_temp_file(othername, &temp[1], two);
- if (! atexit_asked &&
- (temp[0].name == temp[0].tmp_path ||
- temp[1].name == temp[1].tmp_path)) {
- atexit_asked = 1;
- atexit(remove_tempfile);
- }
- signal(SIGINT, remove_tempfile_on_signal);
- }
-
if (one && two) {
+ struct diff_tempfile *temp_one, *temp_two;
+ const char *othername = (other ? other : name);
+ temp_one = prepare_temp_file(name, one);
+ temp_two = prepare_temp_file(othername, two);
*arg++ = pgm;
*arg++ = name;
- *arg++ = temp[0].name;
- *arg++ = temp[0].hex;
- *arg++ = temp[0].mode;
- *arg++ = temp[1].name;
- *arg++ = temp[1].hex;
- *arg++ = temp[1].mode;
+ *arg++ = temp_one->name;
+ *arg++ = temp_one->hex;
+ *arg++ = temp_one->mode;
+ *arg++ = temp_two->name;
+ *arg++ = temp_two->hex;
+ *arg++ = temp_two->mode;
if (other) {
*arg++ = other;
*arg++ = xfrm_msg;
@@ -3450,15 +3455,15 @@ void diff_unmerge(struct diff_options *options,
static char *run_textconv(const char *pgm, struct diff_filespec *spec,
size_t *outsize)
{
- struct diff_tempfile temp;
+ struct diff_tempfile *temp;
const char *argv[3];
const char **arg = argv;
struct child_process child;
struct strbuf buf = STRBUF_INIT;
- prepare_temp_file(spec->path, &temp, spec);
+ temp = prepare_temp_file(spec->path, spec);
*arg++ = pgm;
- *arg++ = temp.name;
+ *arg++ = temp->name;
*arg = NULL;
memset(&child, 0, sizeof(child));
@@ -3467,13 +3472,11 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec,
if (start_command(&child) != 0 ||
strbuf_read(&buf, child.out, 0) < 0 ||
finish_command(&child) != 0) {
- if (temp.name == temp.tmp_path)
- unlink(temp.name);
+ remove_tempfile();
error("error running textconv command '%s'", pgm);
return NULL;
}
- if (temp.name == temp.tmp_path)
- unlink(temp.name);
+ remove_tempfile();
return strbuf_detach(&buf, outsize);
}
--
1.6.1.403.g6c435
^ permalink raw reply related
* [PATCH v2 3/5] chain kill signals for cleanup functions
From: Jeff King @ 2009-01-22 6:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <20090122042643.GB31427@coredump.intra.peff.net>
If a piece of code wanted to do some cleanup before exiting
(e.g., cleaning up a lockfile or a tempfile), our usual
strategy was to install a signal handler that did something
like this:
do_cleanup(); /* actual work */
signal(signo, SIG_DFL); /* restore previous behavior */
raise(signo); /* deliver signal, killing ourselves */
For a single handler, this works fine. However, if we want
to clean up two _different_ things, we run into a problem.
The most recently installed handler will run, but when it
removes itself as a handler, it doesn't put back the first
handler.
This patch introduces sigchain, a tiny library for handling
a stack of signal handlers. You sigchain_push each handler,
and use sigchain_pop to restore whoever was before you in
the stack.
Signed-off-by: Jeff King <peff@peff.net>
---
Two changes since last time:
- rebased on new 2/5, which fixes a problem the original had in calling
sigchain_push every time you did an external diff
- I tried to handle all systems in the test script, which should
hopefully now pass on Windows.
.gitignore | 1 +
Makefile | 3 +++
builtin-clone.c | 5 +++--
builtin-fetch--tool.c | 5 +++--
builtin-fetch.c | 5 +++--
diff.c | 5 +++--
http-push.c | 11 ++++++-----
lockfile.c | 13 +++++++------
sigchain.c | 43 +++++++++++++++++++++++++++++++++++++++++++
sigchain.h | 9 +++++++++
t/t0005-signals.sh | 22 ++++++++++++++++++++++
test-sigchain.c | 22 ++++++++++++++++++++++
12 files changed, 125 insertions(+), 19 deletions(-)
create mode 100644 sigchain.c
create mode 100644 sigchain.h
create mode 100755 t/t0005-signals.sh
create mode 100644 test-sigchain.c
diff --git a/.gitignore b/.gitignore
index d9adce5..f28a54d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -152,6 +152,7 @@ test-match-trees
test-parse-options
test-path-utils
test-sha1
+test-sigchain
common-cmds.h
*.tar.gz
*.dsc
diff --git a/Makefile b/Makefile
index 270b223..30371d1 100644
--- a/Makefile
+++ b/Makefile
@@ -388,6 +388,7 @@ LIB_H += revision.h
LIB_H += run-command.h
LIB_H += sha1-lookup.h
LIB_H += sideband.h
+LIB_H += sigchain.h
LIB_H += strbuf.h
LIB_H += tag.h
LIB_H += transport.h
@@ -481,6 +482,7 @@ LIB_OBJS += sha1-lookup.o
LIB_OBJS += sha1_name.o
LIB_OBJS += shallow.o
LIB_OBJS += sideband.o
+LIB_OBJS += sigchain.o
LIB_OBJS += strbuf.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
@@ -1365,6 +1367,7 @@ TEST_PROGRAMS += test-match-trees$X
TEST_PROGRAMS += test-parse-options$X
TEST_PROGRAMS += test-path-utils$X
TEST_PROGRAMS += test-sha1$X
+TEST_PROGRAMS += test-sigchain$X
all:: $(TEST_PROGRAMS)
diff --git a/builtin-clone.c b/builtin-clone.c
index f7e5a7b..849cefc 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -19,6 +19,7 @@
#include "strbuf.h"
#include "dir.h"
#include "pack-refs.h"
+#include "sigchain.h"
/*
* Overall FIXMEs:
@@ -288,7 +289,7 @@ static void remove_junk(void)
static void remove_junk_on_signal(int signo)
{
remove_junk();
- signal(SIGINT, SIG_DFL);
+ sigchain_pop(signo);
raise(signo);
}
@@ -441,7 +442,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
junk_git_dir = git_dir;
atexit(remove_junk);
- signal(SIGINT, remove_junk_on_signal);
+ sigchain_push(SIGINT, remove_junk_on_signal);
setenv(CONFIG_ENVIRONMENT, xstrdup(mkpath("%s/config", git_dir)), 1);
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 469b07e..b1d7f8f 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -2,6 +2,7 @@
#include "cache.h"
#include "refs.h"
#include "commit.h"
+#include "sigchain.h"
static char *get_stdin(void)
{
@@ -186,7 +187,7 @@ static void remove_keep(void)
static void remove_keep_on_signal(int signo)
{
remove_keep();
- signal(SIGINT, SIG_DFL);
+ sigchain_pop(signo);
raise(signo);
}
@@ -245,7 +246,7 @@ static int fetch_native_store(FILE *fp,
char buffer[1024];
int err = 0;
- signal(SIGINT, remove_keep_on_signal);
+ sigchain_push(SIGINT, remove_keep_on_signal);
atexit(remove_keep);
while (fgets(buffer, sizeof(buffer), stdin)) {
diff --git a/builtin-fetch.c b/builtin-fetch.c
index de6f307..8c86974 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -10,6 +10,7 @@
#include "transport.h"
#include "run-command.h"
#include "parse-options.h"
+#include "sigchain.h"
static const char * const builtin_fetch_usage[] = {
"git fetch [options] [<repository> <refspec>...]",
@@ -58,7 +59,7 @@ static void unlock_pack(void)
static void unlock_pack_on_signal(int signo)
{
unlock_pack();
- signal(SIGINT, SIG_DFL);
+ sigchain_pop(signo);
raise(signo);
}
@@ -672,7 +673,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
ref_nr = j;
}
- signal(SIGINT, unlock_pack_on_signal);
+ sigchain_push(SIGINT, unlock_pack_on_signal);
atexit(unlock_pack);
exit_code = do_fetch(transport,
parse_fetch_refspec(ref_nr, refs), ref_nr);
diff --git a/diff.c b/diff.c
index ae6d552..dacd5d2 100644
--- a/diff.c
+++ b/diff.c
@@ -12,6 +12,7 @@
#include "run-command.h"
#include "utf8.h"
#include "userdiff.h"
+#include "sigchain.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -190,7 +191,7 @@ static void remove_tempfile(void)
static void remove_tempfile_on_signal(int signo)
{
remove_tempfile();
- signal(SIGINT, SIG_DFL);
+ sigchain_pop(signo);
raise(signo);
}
@@ -1904,7 +1905,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (!remove_tempfile_installed) {
atexit(remove_tempfile);
- signal(SIGINT, remove_tempfile_on_signal);
+ sigchain_push(SIGINT, remove_tempfile_on_signal);
remove_tempfile_installed = 1;
}
diff --git a/http-push.c b/http-push.c
index cb5bf95..4c92f80 100644
--- a/http-push.c
+++ b/http-push.c
@@ -10,6 +10,7 @@
#include "exec_cmd.h"
#include "remote.h"
#include "list-objects.h"
+#include "sigchain.h"
#include <expat.h>
@@ -1364,7 +1365,7 @@ static void remove_locks(void)
static void remove_locks_on_signal(int signo)
{
remove_locks();
- signal(signo, SIG_DFL);
+ sigchain_pop(signo);
raise(signo);
}
@@ -2266,10 +2267,10 @@ int main(int argc, char **argv)
goto cleanup;
}
- signal(SIGINT, remove_locks_on_signal);
- signal(SIGHUP, remove_locks_on_signal);
- signal(SIGQUIT, remove_locks_on_signal);
- signal(SIGTERM, remove_locks_on_signal);
+ sigchain_push(SIGINT, remove_locks_on_signal);
+ sigchain_push(SIGHUP, remove_locks_on_signal);
+ sigchain_push(SIGQUIT, remove_locks_on_signal);
+ sigchain_push(SIGTERM, remove_locks_on_signal);
/* Check whether the remote has server info files */
remote->can_update_info_refs = 0;
diff --git a/lockfile.c b/lockfile.c
index 8589155..3cd57dc 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -2,6 +2,7 @@
* Copyright (c) 2005, Junio C Hamano
*/
#include "cache.h"
+#include "sigchain.h"
static struct lock_file *lock_file_list;
static const char *alternate_index_output;
@@ -24,7 +25,7 @@ static void remove_lock_file(void)
static void remove_lock_file_on_signal(int signo)
{
remove_lock_file();
- signal(signo, SIG_DFL);
+ sigchain_pop(signo);
raise(signo);
}
@@ -136,11 +137,11 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 <= lk->fd) {
if (!lock_file_list) {
- signal(SIGINT, remove_lock_file_on_signal);
- signal(SIGHUP, remove_lock_file_on_signal);
- signal(SIGTERM, remove_lock_file_on_signal);
- signal(SIGQUIT, remove_lock_file_on_signal);
- signal(SIGPIPE, remove_lock_file_on_signal);
+ sigchain_push(SIGINT, remove_lock_file_on_signal);
+ sigchain_push(SIGHUP, remove_lock_file_on_signal);
+ sigchain_push(SIGTERM, remove_lock_file_on_signal);
+ sigchain_push(SIGQUIT, remove_lock_file_on_signal);
+ sigchain_push(SIGPIPE, remove_lock_file_on_signal);
atexit(remove_lock_file);
}
lk->owner = getpid();
diff --git a/sigchain.c b/sigchain.c
new file mode 100644
index 0000000..a18d505
--- /dev/null
+++ b/sigchain.c
@@ -0,0 +1,43 @@
+#include "sigchain.h"
+#include "cache.h"
+
+#define SIGCHAIN_MAX_SIGNALS 32
+
+struct sigchain_signal {
+ sigchain_fun *old;
+ int n;
+ int alloc;
+};
+static struct sigchain_signal signals[SIGCHAIN_MAX_SIGNALS];
+
+static void check_signum(int sig)
+{
+ if (sig < 1 || sig >= SIGCHAIN_MAX_SIGNALS)
+ die("BUG: signal out of range: %d", sig);
+}
+
+int sigchain_push(int sig, sigchain_fun f)
+{
+ struct sigchain_signal *s = signals + sig;
+ check_signum(sig);
+
+ ALLOC_GROW(s->old, s->n + 1, s->alloc);
+ s->old[s->n] = signal(sig, f);
+ if (s->old[s->n] == SIG_ERR)
+ return -1;
+ s->n++;
+ return 0;
+}
+
+int sigchain_pop(int sig)
+{
+ struct sigchain_signal *s = signals + sig;
+ check_signum(sig);
+ if (s->n < 1)
+ return 0;
+
+ if (signal(sig, s->old[s->n - 1]) == SIG_ERR)
+ return -1;
+ s->n--;
+ return 0;
+}
diff --git a/sigchain.h b/sigchain.h
new file mode 100644
index 0000000..254ebb0
--- /dev/null
+++ b/sigchain.h
@@ -0,0 +1,9 @@
+#ifndef SIGCHAIN_H
+#define SIGCHAIN_H
+
+typedef void (*sigchain_fun)(int);
+
+int sigchain_push(int sig, sigchain_fun f);
+int sigchain_pop(int sig);
+
+#endif /* SIGCHAIN_H */
diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh
new file mode 100755
index 0000000..9707af7
--- /dev/null
+++ b/t/t0005-signals.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+test_description='signals work as we expect'
+. ./test-lib.sh
+
+cat >expect <<EOF
+three
+two
+one
+EOF
+
+test_expect_success 'sigchain works' '
+ test-sigchain >actual
+ case "$?" in
+ 130) true ;; # POSIX w/ SIGINT=2
+ 3) true ;; # Windows
+ *) false ;;
+ esac &&
+ test_cmp expect actual
+'
+
+test_done
diff --git a/test-sigchain.c b/test-sigchain.c
new file mode 100644
index 0000000..8747dea
--- /dev/null
+++ b/test-sigchain.c
@@ -0,0 +1,22 @@
+#include "sigchain.h"
+#include "cache.h"
+
+#define X(f) \
+static void f(int sig) { \
+ puts(#f); \
+ fflush(stdout); \
+ sigchain_pop(sig); \
+ raise(sig); \
+}
+X(one)
+X(two)
+X(three)
+#undef X
+
+int main(int argc, char **argv) {
+ sigchain_push(SIGINT, one);
+ sigchain_push(SIGINT, two);
+ sigchain_push(SIGINT, three);
+ raise(SIGINT);
+ return 0;
+}
--
1.6.1.403.g6c435
^ permalink raw reply related
* [PATCH v2 4/5] refactor signal handling for cleanup functions
From: Jeff King @ 2009-01-22 6:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <20090122042643.GB31427@coredump.intra.peff.net>
The current code is very inconsistent about which signals
are caught for doing cleanup of temporary files and lock
files. Some callsites checked only SIGINT, while others
checked a variety of death-dealing signals.
This patch factors out those signals to a single function,
and then calls it everywhere. For some sites, that means
this is a simple clean up. For others, it is an improvement
in that they will now properly clean themselves up after a
larger variety of signals.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before, but needed rebasing due to previous diff.c changes.
builtin-clone.c | 2 +-
builtin-fetch--tool.c | 2 +-
builtin-fetch.c | 2 +-
diff.c | 2 +-
http-push.c | 5 +----
lockfile.c | 6 +-----
sigchain.c | 9 +++++++++
sigchain.h | 2 ++
8 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index 849cefc..313df6a 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -442,7 +442,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
junk_git_dir = git_dir;
atexit(remove_junk);
- sigchain_push(SIGINT, remove_junk_on_signal);
+ sigchain_push_common(remove_junk_on_signal);
setenv(CONFIG_ENVIRONMENT, xstrdup(mkpath("%s/config", git_dir)), 1);
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index b1d7f8f..29356d2 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -246,7 +246,7 @@ static int fetch_native_store(FILE *fp,
char buffer[1024];
int err = 0;
- sigchain_push(SIGINT, remove_keep_on_signal);
+ sigchain_push_common(remove_keep_on_signal);
atexit(remove_keep);
while (fgets(buffer, sizeof(buffer), stdin)) {
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 8c86974..1e4a3d9 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -673,7 +673,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
ref_nr = j;
}
- sigchain_push(SIGINT, unlock_pack_on_signal);
+ sigchain_push_common(unlock_pack_on_signal);
atexit(unlock_pack);
exit_code = do_fetch(transport,
parse_fetch_refspec(ref_nr, refs), ref_nr);
diff --git a/diff.c b/diff.c
index dacd5d2..715709b 100644
--- a/diff.c
+++ b/diff.c
@@ -1905,7 +1905,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
if (!remove_tempfile_installed) {
atexit(remove_tempfile);
- sigchain_push(SIGINT, remove_tempfile_on_signal);
+ sigchain_push_common(remove_tempfile_on_signal);
remove_tempfile_installed = 1;
}
diff --git a/http-push.c b/http-push.c
index 4c92f80..178af75 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2267,10 +2267,7 @@ int main(int argc, char **argv)
goto cleanup;
}
- sigchain_push(SIGINT, remove_locks_on_signal);
- sigchain_push(SIGHUP, remove_locks_on_signal);
- sigchain_push(SIGQUIT, remove_locks_on_signal);
- sigchain_push(SIGTERM, remove_locks_on_signal);
+ sigchain_push_common(remove_locks_on_signal);
/* Check whether the remote has server info files */
remote->can_update_info_refs = 0;
diff --git a/lockfile.c b/lockfile.c
index 3cd57dc..021c337 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -137,11 +137,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 <= lk->fd) {
if (!lock_file_list) {
- sigchain_push(SIGINT, remove_lock_file_on_signal);
- sigchain_push(SIGHUP, remove_lock_file_on_signal);
- sigchain_push(SIGTERM, remove_lock_file_on_signal);
- sigchain_push(SIGQUIT, remove_lock_file_on_signal);
- sigchain_push(SIGPIPE, remove_lock_file_on_signal);
+ sigchain_push_common(remove_lock_file_on_signal);
atexit(remove_lock_file);
}
lk->owner = getpid();
diff --git a/sigchain.c b/sigchain.c
index a18d505..1118b99 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -41,3 +41,12 @@ int sigchain_pop(int sig)
s->n--;
return 0;
}
+
+void sigchain_push_common(sigchain_fun f)
+{
+ sigchain_push(SIGINT, f);
+ sigchain_push(SIGHUP, f);
+ sigchain_push(SIGTERM, f);
+ sigchain_push(SIGQUIT, f);
+ sigchain_push(SIGPIPE, f);
+}
diff --git a/sigchain.h b/sigchain.h
index 254ebb0..618083b 100644
--- a/sigchain.h
+++ b/sigchain.h
@@ -6,4 +6,6 @@ typedef void (*sigchain_fun)(int);
int sigchain_push(int sig, sigchain_fun f);
int sigchain_pop(int sig);
+void sigchain_push_common(sigchain_fun f);
+
#endif /* SIGCHAIN_H */
--
1.6.1.403.g6c435
^ permalink raw reply related
* [PATCH v2 5/5] pager: do wait_for_pager on signal death
From: Jeff King @ 2009-01-22 6:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <20090122042643.GB31427@coredump.intra.peff.net>
Since ea27a18 (spawn pager via run_command interface), the
original git process actually does git work, and the pager
is a child process (actually, on Windows it has always been
that way, since Windows lacks fork). After spawning the
pager, we register an atexit() handler that waits for the
pager to finish.
Unfortunately, that handler does not always run. In
particular, if git is killed by a signal, then we exit
immediately. The calling shell then thinks that git is done;
however, the pager is still trying to run and impact the
terminal. The result can be seen by running a long git
process with a pager (e.g., "git log -p") and hitting ^C.
Depending on your config, you should see the shell prompt,
but pressing a key causes the pager to do any terminal
de-initialization sequence.
This patch just intercepts any death-dealing signals and
waits for the pager before dying. Under typical less
configuration, that means hitting ^C will cause git to stop
generating output, but the pager will keep running.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.
| 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
--git a/pager.c b/pager.c
index f19ddbc..4921843 100644
--- a/pager.c
+++ b/pager.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "run-command.h"
+#include "sigchain.h"
/*
* This is split up from the rest of git so that we can do
@@ -38,6 +39,13 @@ static void wait_for_pager(void)
finish_command(&pager_process);
}
+static void wait_for_pager_signal(int signo)
+{
+ wait_for_pager();
+ sigchain_pop(signo);
+ raise(signo);
+}
+
void setup_pager(void)
{
const char *pager = getenv("GIT_PAGER");
@@ -75,6 +83,7 @@ void setup_pager(void)
close(pager_process.in);
/* this makes sure that the parent terminates after the pager */
+ sigchain_push_common(wait_for_pager_signal);
atexit(wait_for_pager);
}
--
1.6.1.403.g6c435
^ permalink raw reply related
* git on the reg
From: David Bryson @ 2009-01-22 7:17 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 405 bytes --]
Hi all,
The regsiter has an article on the guerilla git movement inside
companies. I recall at the GitTogether folks from big pharma, Google,
and Facebook were 'unofficially' using git(not to mention myself).
http://www.theregister.co.uk/2009/01/21/git_gaining_ground/
It is very cool to see this bottom up movement gaining some notariety.
Oh an the Linus jokes are fairly amusing as well ;-).
Dave
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Short "git commit $file" syntax fails in the face of a resolved conflict
From: Johannes Sixt @ 2009-01-22 7:28 UTC (permalink / raw)
To: Nathan Yergler; +Cc: Michael J Gruber, Asheesh Laroia, git
In-Reply-To: <c1a864630901211346j4b702fb3tcc5a098ed7e1541d@mail.gmail.com>
Please don't top-post.
Nathan Yergler schrieb:
> Can you elaborate on why doing -i automatically is a bad idea in this
> case? [It may really be, I don't pretend to have enough knowledge
> about git's internals to make a reasoned argument.] This was
> unexpected behavior for me as I'd always experienced "git add path &&
> git commit" and "git commit path" as being equivalent and so I assumed
> they would work equivalently in this situation.
They are not equivalent. 'git add path && git commit' commits changes to
path *in addition* to what is already staged before you run this command
sequence. But 'git commit path' commits *only* changes to path, leaving
other changes that might be staged uncommitted.
It may become obvious why the latter behavior is unwanted if a merge is in
progress: The merge left changes (and conflicts) in the index; but with
'git commit path' you say that you are not interested in what the index has.
-- Hannes
^ permalink raw reply
* Re: CR codes from git commands
From: Brent Goodrick @ 2009-01-22 7:34 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.1.00.0901212319310.19665@iabervon.org>
On Wed, Jan 21, 2009 at 8:47 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> It's kind of unclear what you're trying to do here. I'm guessing that
> you're trying to run git with stdio directed to a /dev/tty device, where
> isatty() is true, but which doesn't interpret ASCII control characters as
> such. We're not detecting that you can't use a pager on this, and so you
> have to use PAGER=cat (which might not be a bad idea for things like
> "man", either). With some clues about the environment, we should be able
> to do something about this.
>
> You're also trying to send the progress output to a log file that you can
> look at the end of (presumably in a more capable terminal). It should be
> possible (with an option) to get git to output progress info to a non-tty,
> and not use the CRs if the output isn't a tty.
>
> Or do you want to use a tty that can't handle CRs, and get newlines
> instead of CRs? (If I'd git on the first computer I used, it would have
> printed the progress bar over and over in place and probably torn a hole
> in the paper, but I haven't used that one in over 20 years.)
Hi Daniel,
Ideally, yes I would want no CR's but LF's instead (but others who do
not use my environment may actually like the way it is now, and I seek
not to disturb that use case). I could live without the progress
lines (lines that print repeatedly over in one place on normal
terminals), but adding " 2>&1 | cat" to every command line just to get
the CR's to go away, is non-workable for me.
The environment I'm running git under is the Shell mode inside GNU
Emacs. I can't tell you what type of terminal it is, because I believe
that is defined deep in the guts of Emacs. Having read your reply
above, I'm now wondering whether this is an Emacs issue versus a git
issue. If it is an Emacs issue, then I am truly embarrassed for having
wasted everyones time with it.
Brent
^ permalink raw reply
* Re: CR codes from git commands
From: Daniel Barkalow @ 2009-01-22 7:46 UTC (permalink / raw)
To: Brent Goodrick; +Cc: git
In-Reply-To: <e38bce640901212334v1e672d48t81d5c81fecd929eb@mail.gmail.com>
On Wed, 21 Jan 2009, Brent Goodrick wrote:
> On Wed, Jan 21, 2009 at 8:47 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > It's kind of unclear what you're trying to do here. I'm guessing that
> > you're trying to run git with stdio directed to a /dev/tty device, where
> > isatty() is true, but which doesn't interpret ASCII control characters as
> > such. We're not detecting that you can't use a pager on this, and so you
> > have to use PAGER=cat (which might not be a bad idea for things like
> > "man", either). With some clues about the environment, we should be able
> > to do something about this.
> >
> > You're also trying to send the progress output to a log file that you can
> > look at the end of (presumably in a more capable terminal). It should be
> > possible (with an option) to get git to output progress info to a non-tty,
> > and not use the CRs if the output isn't a tty.
> >
> > Or do you want to use a tty that can't handle CRs, and get newlines
> > instead of CRs? (If I'd git on the first computer I used, it would have
> > printed the progress bar over and over in place and probably torn a hole
> > in the paper, but I haven't used that one in over 20 years.)
>
> Hi Daniel,
>
> Ideally, yes I would want no CR's but LF's instead (but others who do
> not use my environment may actually like the way it is now, and I seek
> not to disturb that use case). I could live without the progress
> lines (lines that print repeatedly over in one place on normal
> terminals), but adding " 2>&1 | cat" to every command line just to get
> the CR's to go away, is non-workable for me.
>
> The environment I'm running git under is the Shell mode inside GNU
> Emacs. I can't tell you what type of terminal it is, because I believe
> that is defined deep in the guts of Emacs. Having read your reply
> above, I'm now wondering whether this is an Emacs issue versus a git
> issue. If it is an Emacs issue, then I am truly embarrassed for having
> wasted everyones time with it.
The terminal type, at least in my version of Emacs, is "dumb", which ought
to be sufficient to tell git that a pager isn't going to be useful is most
cases (might be worthwhile to keep "git log" from eating all your memory,
though), and that using CR to rewrite lines isn't going to work.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] contrib: A script to show diff in new window while editing commit message.
From: Junio C Hamano @ 2009-01-22 7:49 UTC (permalink / raw)
To: Ted Pavlic; +Cc: git
In-Reply-To: <1232596208-7384-1-git-send-email-ted@tedpavlic.com>
Ted Pavlic <ted@tedpavlic.com> writes:
> This new script (contrib/giteditor/giteditor) is an example GIT_EDITOR
> that causes the editor to open the commit message as well as a "git diff
> --cached" in a separate window. This behavior differs from "git commit
> -v" in that the diff can be browsed independently of the commit message
> without having to invoke a split window view in an editor.
>
> This script also detects when "stg edit" is being called and uses "stg
> show" instead. Hence, it implements a kind of "stg edit -v".
>
> This script is highly influenced by the "hgeditor" script distributed
> with the Mercurial SCM.
>
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> ---
>
> This new commit responds to some of the issues brought up by Junio C
> Hemano (and Johannes Schindelin). In particular, it removes the
> paragraph from the commit message discussing how it could be "improved."
>
> Also, this new version uses a "DIFFPIPE" to strip the old commit message
> from the top of the "stg show" output so that the "stg edit" behavior
> matches the "git commit" behavior.
>
> Finally, this version adds a comment giving an idea for personalizing by
> adding the temporary directory creation back in (as a way to prevent
> editor backup files from piling up inside .git).
>
> contrib/giteditor/giteditor | 86 +++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 86 insertions(+), 0 deletions(-)
> create mode 100755 contrib/giteditor/giteditor
>
> diff --git a/contrib/giteditor/giteditor b/contrib/giteditor/giteditor
> new file mode 100755
> index 0000000..5369732
> --- /dev/null
> +++ b/contrib/giteditor/giteditor
> @@ -0,0 +1,86 @@
> +#!/bin/sh
> +#
> +# Set GIT_EDITOR (or core.editor) to this script to see a diff alongside
> +# commit message. This script differs from "git commit -v" in that the
> +# diff shows up in a separate buffer. Additionally, this script works
> +# with "stg edit" as well.
> +#
> +# Copyright (c) 2009 by Theodore P. Pavlic <ted@tedpavlic.com>
> +# Highly influenced by hgeditor script distributed with Mercurial SCM.
> +# Distributed under the GNU General Public License, version 2.0.
> +#
> +# Possible personalizations:
> +#
> +# * If your GIT_DIR gets polluted with backup files created by your
> +# editor when COMMIT_EDITMSG is saved, then have this script copy
> +# COMMIT_EDITMSG (i.e., $1) to a temporary directory and then back to
> +# COMMIT_EDITMSG when done. When the script cleans up after itself, it
> +# can delete the temporary directory and any leftover backup files.
> +# Note that the vim setting 'nobackup' disables saving backup files,
> +# and this setting can be set automatically on gitcommit-type files
> +# and files matching .stgit-*.txt with an appropriate ftdetect entry.
I am not sure what problem you are trying to offer a solution here. Are
you suggesting a trick to avoid .git/COMMIT_EDITMSG~ left behind by Emacs?
> +# Find git
> +test -z "${GIT}" && GIT="git"
Why?
> +# Find stg
> +test -z "${STG}" && STG="stg"
Why?
> +# Find the nearest git-dir
> +GITDIR=$(git rev-parse --git-dir) || exit
> +
> +# Use an editor. To prevent loops, avoid GIT_EDITOR and core.editor.
> +EDITOR="${GIT_EDITOR_EDITOR-${VISUAL-${EDITOR-vi}}}"
At the beginning of this file, you have a nice insn to set GIT_EDITOR, but
this GIT_EDITOR_EDITOR to customize what underlying editor you end up
launching should also be documented in the same place.
I do not think you need the dq pair around the right hand side.
> +# If we recognize a popular editor, add necessary flags (e.g., to
> +# prevent forking)
> +case "${EDITOR}" in
> + emacs)
> + EDITOR="${EDITOR} -nw"
> + ;;
> + mvim|gvim|vim|vi)
> + EDITOR="${EDITOR} -f -o"
> + ;;
> +esac
- Please align case arm labels with case and esac, like this (I do not
mind 4-space indentation if that is what you are used to):
case "$foo" in
bar)
... do bar things ...
;;
...
esac
- Braces around variables are noisy and distracting;
- Why force emacs users to -nw? If _you_ personally like -nw, shouldn't
you be able to simply do:
GIT_EDITOR_EDITOR="emacs -nw"
in your environment? After all, when you use $EDITOR later, you do not
quote it and let $IFS separate the flags the variable may have in
addition to the name of (or path to) the executable.
> +# Remove temporary files even if we get interrupted
> +DIFFOUTPUT="${GITDIR}/giteditor.${RANDOM}.${RANDOM}.${RANDOM}.$$.diff"
The script begins with #!/bin/sh but isn't ${RANDOM} a Bash-ism? Either
you should begin it with #!/bin/bash, or avoid bash-ism, if you do not
want to alienate poeple whose /bin/sh is not bash.
> +cleanup_exit() {
> + rm -f "${DIFFOUTPUT}"
> +}
> +trap "cleanup_exit" 0 # normal exit
> +trap "exit 255" 1 2 3 6 15 # HUP INT QUIT ABRT TERM
It may have been useful while debugging this script, but is the temporary
file that precious to it needs to be kept upon HUP and friends?
> +# For git, COMMITMSG=COMMIT_EDITMSG
> +# For stg, COMMITMSG=.stgit-edit.txt
> +# etc.
> +COMMITMSG=$(basename "$1")
> +DIFFPIPE="cat"
> +case "${COMMITMSG}" in
> + .stgit-edit.txt) # From "stg edit"
> + DIFFCMD="${STG}"
> + DIFFARGS="show"
> + DIFFPIPE="tail +$(wc -l "$1"|awk '{print $1+3}')"
> + ;;
> + *) # Fall through to "git commit" case
> + DIFFCMD="${GIT}"
> + DIFFARGS="diff --cached"
> + # To focus on files that changed, use:
> + #DIFFARGS="diff --cached --diff-filter=M"
> + ;;
> +esac
> +
This '*' case is horribly wrong.
What happens if other parts of git (or third party tools around git) runs
core.editor for things other than the commit log messages (or anything you
do not know how to prepare a sensible diff to show)? Shouldn't you act as
if you were not there at all to avoid surprising the end user with an
unexpected diff output?
For example, doesn't "git rebase -i" launch core.editor to let you edit
the pick/edit/squash insn sequence? What diff are you showing in such a
case, and how would that help your users?
> +# Do the diff and save the result in DIFFOUTPUT
> +"${DIFFCMD}" ${DIFFARGS} | ${DIFFPIPE} > ${DIFFOUTPUT}
Because your "extra information in another file" processing depends
heavily on what the edited file is (you are already generating the diff
differently for "git commit" and "stg edit"), I think having this outside
the above case statement is a false factoring; it would be easier to read
and maintain if they are defined in each case arm. I would probably write
this part like this:
case "$COMMITMSG" in
.stgit-edit.txt)
do whatever appropriate for "stg edit" and emit to stdout
;;
COMMIT_EDITMSG)
if git rev-parse -q --verify HEAD >/dev/null
then
git diff --cached
else
git diff --cached 4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi
;;
*)
: we do not know how to handle this one
;;
esac >"$DIFFOUTPUT"
You need to quote "$DIFFOUTPUT"; it is a path in $GIT_DIR which means it
can have $IFS character.
> +# If DIFFOUTPUT is nonempty, open it alongside commit message
> +if test -s "${DIFFOUTPUT}"; then
> + # Diff is non-empty, so edit msg and diff
> + ${EDITOR} "$1" "${DIFFOUTPUT}" || exit
> +else
> + # Empty diff. Only edit msg
> + ${EDITOR} "$1" || exit
In the latter case, it may be cleaner to:
rm -f "$DIFFOUTPUT"
exec ${EDITOR} "$1"
One more thing. You may want to study how git_editor() in git-sh-setup
scriptlet solves the issue of (1) the path to the editor executable may
have $IFS character that the end user may want to quote, and (2) the end
user may want to include options to the editor in the varaible, by using
eval.
> +fi
> +
> +# (recall that DIFFOUTPUT file gets cleaned up by trap above)
> +exit
^ permalink raw reply
* lineups with GIT
From: outre @ 2009-01-22 7:54 UTC (permalink / raw)
To: git
I am trying to set up two different line-ups for a project (development and
testing).
The development line-up is the master, and test line-up pulls data from it.
The code is the same in both.
But since the line-ups are served from two different domains one folder in
the development line-up is called
iWeb.local, and in the test line-up it is called iWeb.test. That is the only
difference between the two.
I tried using "git mv" command and it somewhat solved the problem. After I
cloned the devel line-up, I used "git rm iWeb.local iWeb.test".
And now if I edit a file in iWeb.local and do a pull to iWeb.test this file
gets properly updated while preserving the difference between
the folder names. But if I add a new file to iWeb.local, and then do a pull
I get iWeb.local folder added together with the
new file to the testing line-up.
I was wondering if it is intended behaviour for GIT. And if it is may be
someone can point me to a better way to setup two line-ups using
GIT.
Thanks
--
View this message in context: http://n2.nabble.com/lineups-with-GIT-tp2196604p2196604.html
Sent from the git mailing list archive at Nabble.com.
^ 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