Re: Topics currently in the Stalled category
On Wed, Nov 21, 2012 at 12:05 AM, Junio C Hamano wrote: > Here is a list of stalled topics I am having trouble deciding what > to do (the default is to dismiss them around feature freeze). [snipped] > * as/check-ignore (2012-11-08) 14 commits > - t0007: fix tests on Windows > - Documentation/check-ignore: we show the deciding match, not the first > - Add git-check-ignore sub-command > - dir.c: provide free_directory() for reclaiming dir_struct memory > - pathspec.c: move reusable code from builtin/add.c > - dir.c: refactor treat_gitlinks() > - dir.c: keep track of where patterns came from > - dir.c: refactor is_path_excluded() > - dir.c: refactor is_excluded() > - dir.c: refactor is_excluded_from_list() > - dir.c: rename excluded() to is_excluded() > - dir.c: rename excluded_from_list() to is_excluded_from_list() > - dir.c: rename path_excluded() to is_path_excluded() > - dir.c: rename cryptic 'which' variable to more consistent name > > Duy helped to reroll this, but it seems that there weren't any > activity since then during my absense. I have been delayed several times, but I finally resumed work on another re-roll. I don't think there is any major reworking required; just a number of small tweaks. > * as/test-tweaks (2012-09-20) 7 commits > - tests: paint unexpectedly fixed known breakages in bold red > - tests: test the test framework more thoroughly > - [SQUASH] t/t-basic.sh: quoting of TEST_DIRECTORY is screwed up > - tests: refactor mechanics of testing in a sub test-lib > - tests: paint skipped tests in bold blue > - tests: test number comes first in 'not ok $count - $message' > - tests: paint known breakages in bold yellow > > Various minor tweaks to the test framework to paint its output > lines in colors that match what they mean better. > > Has the "is this really blue?" issue Peff raised resolved??? I have a re-roll of this ready - just need to rebase to latest master, do a final sanity check, and then send. Sorry again for the delays. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] remove/deprecate 'submodule init' and 'sync'
On Fri, Nov 30, 2012 at 12:53 PM, W. Trevor King wrote: > On Wed, Nov 28, 2012 at 12:19:04AM +0100, Jens Lehmann wrote: >> Am 26.11.2012 22:00, schrieb W. Trevor King: >> > From: "W. Trevor King" >> > >> > This allows users to override the .gitmodules value with a >> > per-repository value. >> >> Your intentions makes lots of sense, but your patch does more than >> that. Copying the branch setting into .git/config sets the initial >> branch setting into stone. That makes it impossible to have a branch >> "foo" in the superproject using a branch "bar" in a submodule and >> another superproject branch "frotz" using branch "nitfol" for the >> same submodule. You should use the branch setting from .git/config >> if present and fall back to the branch setting from .gitmodules if >> not, which would enable the user to have her own setting if she >> doesn't like what upstream provides but would still enable others >> to follow different submodule branches in different superproject >> branches. > > I've mulling this over, and when I started coding support for > submodule..remote, I had an idea. > > On Thu, Nov 29, 2012 at 10:27:19PM -0500, W. Trevor King wrote: >> On Thu, Nov 29, 2012 at 08:11:20PM -0500, Phil Hord wrote: >> > I've always felt that the "origin" defaults are broken and are simply >> > being ignored because most users do not trip over them. But ISTR that >> > submodule commands use the remote indicated by the superproject's >> > current remote-tracking configuration, with a fallback to 'origin' if >> > there is none. Sort of a "best effort" algorithm, I think. Am I >> > remembering that wrong? >> >> The current code uses a bare "git-fetch". I'm not sure what that >> defaults to if you're on a detached head. If it bothers you, I'm fine >> adding the submodule..remote option in v6. > > In my v5 patch, I check for submodule..remote first in the usual > `git config` files. If I don't find what I'm looking for I fall back > on .gitmodules (basically Jens' suggestion). However, my initial > copying-to-.git/config approach was mostly done to mimic existing > configuration handling in git-submodule.sh. Since I agree with Jens > on configuration precendence, and I now had two options to read > (.branch and .remote), I thought I'd pull the logic out into its own > function (code included at the end). While I was shifting the > existing submodule config handling over to my new function, I noticed > that with this logic, `submodule init` doesn't really do anything > important anymore. If I never 'submodule init' a submodule, it does not get visited by 'git submodule foreach', among others. I think some people use this behavior explicitly. On the other hand, I've also notice that a submodule which I have removed does not get de-inited later one. It causes my 'git submodule foreach' to emit errors. :-( Phil > Likewise for `submodule sync`, which seems to be > quite similar to `init`. > > What to do about this? `init` has been around for a while, so we > can't just remove it (maybe in 2.0?). Leaving it in place is not > really a problem though, it just means that the user is locking in the > current .gitmodules configuration (as Jens pointed out with respect to > .branch). > > I may be way off base here, as I'm fairly new to submodules in general > and these two commands in particular, but I thought I'd float the > idea. > > Cheers, > Trevor > > --- > # > # Print a submodule configuration setting > # > # $1 = submodule name > # $2 = option name > # $3 = default value > # > # Checks in the usual git-config places first (for overrides), > # otherwise it falls back on .gitmodules. This allows you to > # distribute project-wide defaults in .gitmodules, while still > # customizing individual repositories if necessary. If the option is > # not in .gitmodules either, print a default value. > # > get_submodule_config() > { > name="$1" > option="$2" > default="$3" > value=$(git config submodule."$name"."$option") > if test -z "$value" > then > value=$(git config -f .gitmodules submodule."$name"."$option") > fi > printf '%s' "${value:-$default}" > } > > -- > This email may be signed or encrypted with GnuPG (http://www.gnupg.org). > For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] launch_editor: propagate signals from editor to git
We block SIGINT and SIGQUIT while the editor runs so that git is not killed accidentally by a stray "^C" meant for the editor or its subprocesses. This works because most editors ignore SIGINT. However, some editor wrappers, like emacsclient, expect to die due to ^C. We detect the signal death in the editor and properly exit, but not before writing a useless error message to stderr. Instead, let's notice when the editor was killed by a terminal signal and just raise the signal on ourselves. This skips the message and looks to our parent like we received SIGINT ourselves. The end effect is that if the user's editor ignores SIGINT, we will, too. And if it does not, then we will behave as if we did not ignore it. That should make all users happy. Note that in the off chance that another part of git has ignored SIGINT while calling launch_editor, we will still properly detect and propagate the failed return code from the editor (i.e., the worst case is that we generate the useless error, not fail to notice the editor's death). Signed-off-by: Jeff King --- editor.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/editor.c b/editor.c index c892a81..065a7ab 100644 --- a/editor.c +++ b/editor.c @@ -39,7 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; struct child_process p; - int ret; + int ret, sig; memset(&p, 0, sizeof(p)); p.argv = args; @@ -51,8 +51,11 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en sigchain_push(SIGINT, SIG_IGN); sigchain_push(SIGQUIT, SIG_IGN); ret = finish_command(&p); + sig = ret + 128; sigchain_pop(SIGINT); sigchain_pop(SIGQUIT); + if (sig == SIGINT || sig == SIGQUIT) + raise(sig); if (ret) return error("There was a problem with the editor '%s'.", editor); -- 1.8.0.1.620.g558b0aa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] run-command: do not warn about child death from terminal
SIGINT and SIGQUIT are not generally interesting signals to the user, since they are typically caused by them hitting "^C" or otherwise telling their terminal to send the signal. Signed-off-by: Jeff King --- run-command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index 3aae270..757f263 100644 --- a/run-command.c +++ b/run-command.c @@ -242,7 +242,8 @@ static int wait_or_whine(pid_t pid, const char *argv0) error("waitpid is confused (%s)", argv0); } else if (WIFSIGNALED(status)) { code = WTERMSIG(status); - error("%s died of signal %d", argv0, code); + if (code != SIGINT && code != SIGQUIT) + error("%s died of signal %d", argv0, code); /* * This return value is chosen so that code & 0xff * mimics the exit code that a POSIX shell would report for -- 1.8.0.1.620.g558b0aa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] launch_editor: ignore terminal signals while editor has control
From: Paul Fox The user's editor likely catches SIGINT (ctrl-C). but if the user spawns a command from the editor and uses ctrl-C to kill that command, the SIGINT will likely also kill git itself (depending on the editor, this can leave the terminal in an unusable state). Let's ignore it while the editor is running, and do the same for SIGQUIT, which many editors also ignore. This matches the behavior if we were to use system(3) instead of run-command. Signed-off-by: Paul Fox Signed-off-by: Jeff King --- editor.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/editor.c b/editor.c index 842f782..c892a81 100644 --- a/editor.c +++ b/editor.c @@ -1,6 +1,7 @@ #include "cache.h" #include "strbuf.h" #include "run-command.h" +#include "sigchain.h" #ifndef DEFAULT_EDITOR #define DEFAULT_EDITOR "vi" @@ -38,6 +39,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; struct child_process p; + int ret; memset(&p, 0, sizeof(p)); p.argv = args; @@ -46,7 +48,12 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (start_command(&p) < 0) return error("unable to start editor '%s'", editor); - if (finish_command(&p)) + sigchain_push(SIGINT, SIG_IGN); + sigchain_push(SIGQUIT, SIG_IGN); + ret = finish_command(&p); + sigchain_pop(SIGINT); + sigchain_pop(SIGQUIT); + if (ret) return error("There was a problem with the editor '%s'.", editor); } -- 1.8.0.1.620.g558b0aa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] launch_editor: refactor to use start/finish_command
The launch_editor function uses the convenient run_command_* interface. Let's use the more flexible start_command and finish_command functions, which will let us manipulate the parent state while we're waiting for the child to finish. Signed-off-by: Jeff King --- editor.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/editor.c b/editor.c index d834003..842f782 100644 --- a/editor.c +++ b/editor.c @@ -37,8 +37,16 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (strcmp(editor, ":")) { const char *args[] = { editor, path, NULL }; + struct child_process p; - if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env)) + memset(&p, 0, sizeof(p)); + p.argv = args; + p.env = env; + p.use_shell = 1; + if (start_command(&p) < 0) + return error("unable to start editor '%s'", editor); + + if (finish_command(&p)) return error("There was a problem with the editor '%s'.", editor); } -- 1.8.0.1.620.g558b0aa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] run-command: drop silent_exec_failure arg from wait_or_whine
We do not actually use this parameter; instead we complain from the child itself (for fork/exec) or from start_command (if we are using spawn on Windows). Signed-off-by: Jeff King --- run-command.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/run-command.c b/run-command.c index 3b982e4..3aae270 100644 --- a/run-command.c +++ b/run-command.c @@ -226,7 +226,7 @@ static inline void set_cloexec(int fd) fcntl(fd, F_SETFD, flags | FD_CLOEXEC); } -static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure) +static int wait_or_whine(pid_t pid, const char *argv0) { int status, code = -1; pid_t waiting; @@ -432,8 +432,7 @@ fail_pipe: * At this point we know that fork() succeeded, but execvp() * failed. Errors have been reported to our stderr. */ - wait_or_whine(cmd->pid, cmd->argv[0], - cmd->silent_exec_failure); + wait_or_whine(cmd->pid, cmd->argv[0]); failed_errno = errno; cmd->pid = -1; } @@ -538,7 +537,7 @@ int finish_command(struct child_process *cmd) int finish_command(struct child_process *cmd) { - return wait_or_whine(cmd->pid, cmd->argv[0], cmd->silent_exec_failure); + return wait_or_whine(cmd->pid, cmd->argv[0]); } int run_command(struct child_process *cmd) -- 1.8.0.1.620.g558b0aa -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] ignore SIG{INT,QUIT} when launching editor
This is a re-roll of the pf/editor-ignore-sigint series. There are two changes from the original: 1. We ignore both SIGINT and SIGQUIT for "least surprise" compared to system(3). 2. We now use "code + 128" to look for signal death (instead of WTERMSIG), as per run-command's documentation on how it munges the code. People mentioned some buggy editors which go into an infinite EIO loop when their parent dies due to SIGQUIT. That should be a non-issue now, as we will be ignoring SIGQUIT. And even if you could replicate it (e.g., with another signal) those programs should be (and reportedly have been) fixed. It is not git's job to babysit its child processes. The patches are: [1/5]: run-command: drop silent_exec_failure arg from wait_or_whine [2/5]: launch_editor: refactor to use start/finish_command [3/5]: launch_editor: ignore terminal signals while editor has control [4/5]: run-command: do not warn about child death from terminal [5/5]: launch_editor: propagate signals from editor to git Since this can be thought of as "act more like system(3)", I wondered whether the signal-ignore logic should be moved into run-command, or even used by default for blocking calls to run_command (which are basically our version of system(3)). But it is detrimental in the common case that the child is not taking control of the terminal, and is just an implementation detail (e.g., we call "git update-ref" behind the scenes, but the user does not know or care). If they hit ^C during such a run and we are ignoring SIGINT, then either: 1. we will notice the child died by signal and report an error in the subprocess rather than just dying; the end result is similar, but the error is unnecessarily confusing 2. we do not bother to check the child's return code (because we do not care whether the child succeeded or not, like a "gc --auto"); we end up totally ignoring the user's request to abort the operation So I do not think we care about this behavior except for launching the editor. And the signal-propagation behavior of 5/5 is really so weirdly editor-specific (because it is about behaving well whether the child blocks signals or not). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitk: add a checkbox to control the visibility of tags
Enable hiding of tags displayed in the tree as yellow labels. If a repository is used together with a system like Gerrit there may be quite a lot of tags used to control building and there may be hardly any place left for commit subjects. Signed-off-by: Łukasz Stelmach --- gitk-git/gitk | 23 +++ 1 files changed, 15 insertions(+), 8 deletions(-) diff --git a/gitk-git/gitk b/gitk-git/gitk index d93bd99..274b46b 100755 --- a/gitk-git/gitk +++ b/gitk-git/gitk @@ -1754,7 +1754,7 @@ proc readrefs {} { global tagids idtags headids idheads tagobjid global otherrefids idotherrefs mainhead mainheadid global selecthead selectheadid -global hideremotes +global hideremotes hidetags foreach v {tagids idtags headids idheads otherrefids idotherrefs} { catch {unset $v} @@ -1776,6 +1776,7 @@ proc readrefs {} { set headids($name) $id lappend idheads($id) $name } elseif {[string match "tags/*" $name]} { + if {$hidetags} continue # this lets refs/tags/foo^{} overwrite refs/tags/foo, # which is what we want since the former is the commit ID set name [string range $name 5 end] @@ -2702,7 +2703,7 @@ proc savestuff {w} { global cmitmode wrapcomment datetimeformat limitdiffs global colors uicolor bgcolor fgcolor diffcolors diffcontext selectbgcolor global autoselect autosellen extdifftool perfile_attrs markbgcolor use_ttk -global hideremotes want_ttk +global hideremotes hidetags want_ttk if {$stuffsaved} return if {![winfo viewable .]} return @@ -2725,6 +2726,7 @@ proc savestuff {w} { puts $f [list set autosellen $autosellen] puts $f [list set showneartags $showneartags] puts $f [list set hideremotes $hideremotes] + puts $f [list set hidetags $hidetags] puts $f [list set showlocalchanges $showlocalchanges] puts $f [list set datetimeformat $datetimeformat] puts $f [list set limitdiffs $limitdiffs] @@ -10864,7 +10866,7 @@ proc create_prefs_page {w} { proc prefspage_general {notebook} { global NS maxwidth maxgraphpct showneartags showlocalchanges global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs -global hideremotes want_ttk have_ttk +global hideremotes hidetags want_ttk have_ttk set page [create_prefs_page $notebook.general] @@ -10887,6 +10889,9 @@ proc prefspage_general {notebook} { ${NS}::checkbutton $page.hideremotes -text [mc "Hide remote refs"] \ -variable hideremotes grid x $page.hideremotes -sticky w +${NS}::checkbutton $page.hidetags -text [mc "Hide tag labels"] \ + -variable hidetags +grid x $page.hidetags -sticky w ${NS}::label $page.ddisp -text [mc "Diff display options"] grid $page.ddisp - -sticky w -pady 10 @@ -10988,7 +10993,7 @@ proc doprefs {} { global oldprefs prefstop showneartags showlocalchanges global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs -global hideremotes want_ttk have_ttk +global hideremotes hidetags want_ttk have_ttk set top .gitkprefs set prefstop $top @@ -10997,7 +11002,7 @@ proc doprefs {} { return } foreach v {maxwidth maxgraphpct showneartags showlocalchanges \ - limitdiffs tabstop perfile_attrs hideremotes want_ttk} { + limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} { set oldprefs($v) [set $v] } ttk_toplevel $top @@ -7,7 +11122,7 @@ proc prefscan {} { global oldprefs prefstop foreach v {maxwidth maxgraphpct showneartags showlocalchanges \ - limitdiffs tabstop perfile_attrs hideremotes want_ttk} { + limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} { global $v set $v $oldprefs($v) } @@ -11131,7 +11136,7 @@ proc prefsok {} { global oldprefs prefstop showneartags showlocalchanges global fontpref mainfont textfont uifont global limitdiffs treediffs perfile_attrs -global hideremotes +global hideremotes hidetags catch {destroy $prefstop} unset prefstop @@ -11177,7 +11182,8 @@ proc prefsok {} { $limitdiffs != $oldprefs(limitdiffs)} { reselectline } -if {$hideremotes != $oldprefs(hideremotes)} { +if {$hideremotes != $oldprefs(hideremotes) || +$hidetags != $oldprefs(hidetags)} { rereadrefs } } @@ -11601,6 +11607,7 @@ set cmitmode "patch" set wrapcomment "none" set showneartags 1 set hideremotes 0 +set hidetags 0 set maxrefs 20 set maxlinelen 200 set showlocalchanges 1 -- 1.7.8.6 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] launch_editor: propagate SIGINT from editor to git
On Sun, Nov 11, 2012 at 08:48:38PM +0100, Johannes Sixt wrote: > Am 11.11.2012 17:57, schrieb Jeff King: > > @@ -51,6 +51,8 @@ int launch_editor(const char *path, struct strbuf > > *buffer, const char *const *en > > sigchain_push(SIGINT, SIG_IGN); > > ret = finish_command(&p); > > sigchain_pop(SIGINT); > > + if (WIFSIGNALED(ret) && WTERMSIG(ret) == SIGINT) > > + raise(SIGINT); > > The return value of finish_command() is already a digested version of > waitpid's status value. According to > Documentation/technical/api-run-command.txt: > > . If the program terminated due to a signal, then the return value is > the signal number - 128, ... > > the correct condition would be > > if (ret == SIGINT - 128) Yeah, that is the same thing as WTERMSIG (which uses "ret & 0x7f") for the range of -127..-1. I do not mind changing it to match run-command's stated output, but I am curious whether there are systems where WTERMSIG is not defined in the same way, and the code would break. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fsck: warn about ".git" in trees
On Fri, Nov 30, 2012 at 08:50:41PM +0100, Torsten Bögershausen wrote: > >Having a ".git" entry inside a tree can cause confusing > >results on checkout. At the top-level, you could not > >checkout such a tree, as it would complain about overwriting > >the real ".git" directory. In a subdirectory, you might > >check it out, but performing operations in the subdirectory > >would confusingly consider the in-tree ".git" directory as > >the repository. > [snip] > >+int has_dotgit = 0; > > Name like "." or ".." are handled as directories by the OS. Right. In theory git could run on a system that does not treat them specially, but in practice they are going to be problematic on most systems. > ".git" could be a file or a directory, at least in theory, and from > the OS point of view, but we want to have this as a reserved name. Exactly. > Looking at bad directory names, which gives trouble when checking out: > > Should we check for "/" or "../blabla" as well? We do already (the error is "contains full pathnames"). We also cover empty pathnames and some other cases. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fsck: warn about ".git" in trees
Having a ".git" entry inside a tree can cause confusing results on checkout. At the top-level, you could not checkout such a tree, as it would complain about overwriting the real ".git" directory. In a subdirectory, you might check it out, but performing operations in the subdirectory would confusingly consider the in-tree ".git" directory as the repository. [snip] + int has_dotgit = 0; Name like "." or ".." are handled as directories by the OS. ".git" could be a file or a directory, at least in theory, and from the OS point of view, but we want to have this as a reserved name. Looking at bad directory names, which gives trouble when checking out: Should we check for "/" or "../blabla" as well? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] win32: support echo for terminal-prompt
On Fri, Nov 30, 2012 at 11:16:59AM +0100, Erik Faye-Lund wrote: > Ping? Thanks for the reminder; your initial series came while I was traveling. I think it looks good. The compat/terminal code ends up a little uglier, but I think you overall did a good job of balancing code reuse across platforms with readability. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 5/5] mingw: get rid of getpass implementation
On Tue, Nov 13, 2012 at 03:04:07PM +0100, Erik Faye-Lund wrote: > There's no remaining call-sites, and as pointed out in the > previous commit message, it's not quite ideal. So let's just > lose it. > > Signed-off-by: Erik Faye-Lund > --- > compat/mingw.c | 15 --- > compat/mingw.h | 2 -- > 2 files changed, 17 deletions(-) Yay! -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 4/5] mingw: reuse tty-version of git_terminal_prompt
On Tue, Nov 13, 2012 at 03:04:06PM +0100, Erik Faye-Lund wrote: > The getpass-implementation we use on Windows isn't at all ideal; > it works in raw-mode (as opposed to cooked mode), and as a result > does not deal correcly with deletion, arrow-keys etc. > > Instead, use cooked mode to read a line at the time, allowing the > C run-time to process the input properly. > > Since we set files to be opened in binary-mode by default on > Windows, introduce a FORCE_TEXT macro that expands to the "t" > modifier that forces the terminal to be opened in text-mode so we > do not have to deal with CRLF issues. I think this is OK. I had originally envisioned just having a separate "#ifdef WIN32" and not really sharing code at all with /dev/tty because I was worried that the conditionals would end up making it hard to read. This is a little more complex than I would have liked, but I do not see how the code sharing could be simplified any more than what you have done, and it is nice to avoid repeating ourselves. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 3/5] compat/terminal: separate input and output handles
On Tue, Nov 13, 2012 at 03:04:05PM +0100, Erik Faye-Lund wrote: > On Windows, the terminal cannot be opened in read-write mode, so > we need distinct pairs for reading and writing. Since this works > fine on other platforms as well, always open them in pairs. Looks OK. We're now opening /dev/tty three separate times in the no-echo case, but it's not like this is in a performance critical loop. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling
On Fri, Nov 30, 2012 at 06:59:30PM +0100, Johannes Schindelin wrote: > > diff --git a/compat/terminal.c b/compat/terminal.c > > index bbb038d..3217838 100644 > > --- a/compat/terminal.c > > +++ b/compat/terminal.c > > @@ -14,6 +14,7 @@ static void restore_term(void) > > return; > > > > tcsetattr(term_fd, TCSAFLUSH, &old_term); > > + close(term_fd); > > term_fd = -1; > > } > > That looks like an independent resource leak fix... correct? I don't think so. In the current code, term_fd does not take ownership of the fd. It is fundamentally part of the FILE* in git_terminal_prompt, and is closed when we fclose() that. But in Erik's refactor, we actually open a _second_ descriptor to /dev/tty, which needs to be closed. I don't think there is any reason that should not work (the terminal characteristics should be per-tty, not per-descriptor), though it does feel a little hacky to have to open /dev/tty twice. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] remove/deprecate 'submodule init' and 'sync'
On Fri, Nov 30, 2012 at 12:53:09PM -0500, W. Trevor King wrote: > Likewise for `submodule sync`, which seems to be > quite similar to `init`. Ah, I'd remove the part of `sync` that touches the superproject's .git/config, but keep the part that stores the superproject-reorded URL in the submodule's config: url=$(get_submodule_config "$name" url) up_path=$(get_up_path "$sm_path") url=$(resolve_relative_url "$url" "$up_path") && if test -n "$url" then if test -e "$sm_path"/.git then ( clear_local_git_env cd "$sm_path" remote=$(get_default_remote) git config remote."$remote".url "$url" ) fi fi I should probably also tweak sync to do similar things with submodule..branch and .remote as part of my `--update remote` series. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
On Fri, Nov 30, 2012 at 06:58:11PM +0100, Johannes Schindelin wrote: > Hi, > > On Tue, 13 Nov 2012, Erik Faye-Lund wrote: > > > Set a control-handler to prevent the process from terminating, and > > simulate SIGINT so it can be handled by a signal-handler as usual. > > One thing you might want to mention is that the fgetc() handling is not > thread-safe, and intentionally so: if two threads read from the same > console, we are in trouble anyway. That makes sense to me, but I'm confused why it is part of mingw_fgetc, which could in theory read from arbitrary streams, no? It it is not necessarily a console operation at all. I feel like I'm probably missing something subtle here... -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH/RFC 5/5] mingw: get rid of getpass implementation
Hi kusma, On Tue, 13 Nov 2012, Erik Faye-Lund wrote: > There's no remaining call-sites, and as pointed out in the > previous commit message, it's not quite ideal. So let's just > lose it. Awesome! Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH/RFC 4/5] mingw: reuse tty-version of git_terminal_prompt
Hi kusma, On Tue, 13 Nov 2012, Erik Faye-Lund wrote: > The getpass-implementation we use on Windows isn't at all ideal; > it works in raw-mode (as opposed to cooked mode), and as a result > does not deal correcly with deletion, arrow-keys etc. > > Instead, use cooked mode to read a line at the time, allowing the > C run-time to process the input properly. Awesome! The patch itself has a couple Windows-specific things in compat/terminal.c that I would have loved to see in compat/mingw.c instead, but it looks as if we have no choice: restore_term() and disable_echo() need to be substantially different enough from the implementation in compat/mingw.c. (Read: I am fine with this patch.) Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling
Hi, On Tue, 13 Nov 2012, Erik Faye-Lund wrote: > By moving the echo-disabling code to a separate function, we can > implement OS-specific versions of it for non-POSIX platforms. > > Signed-off-by: Erik Faye-Lund > --- > compat/terminal.c | 43 +-- > 1 file changed, 25 insertions(+), 18 deletions(-) > > diff --git a/compat/terminal.c b/compat/terminal.c > index bbb038d..3217838 100644 > --- a/compat/terminal.c > +++ b/compat/terminal.c > @@ -14,6 +14,7 @@ static void restore_term(void) > return; > > tcsetattr(term_fd, TCSAFLUSH, &old_term); > + close(term_fd); > term_fd = -1; > } That looks like an independent resource leak fix... correct? Rest looks awsomely fine. Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH/RFC 1/5] mingw: make fgetc raise SIGINT if apropriate
Hi, On Tue, 13 Nov 2012, Erik Faye-Lund wrote: > Set a control-handler to prevent the process from terminating, and > simulate SIGINT so it can be handled by a signal-handler as usual. One thing you might want to mention is that the fgetc() handling is not thread-safe, and intentionally so: if two threads read from the same console, we are in trouble anyway. BTW I like the new mingw_raise() very much! Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] remove/deprecate 'submodule init' and 'sync'
On Wed, Nov 28, 2012 at 12:19:04AM +0100, Jens Lehmann wrote: > Am 26.11.2012 22:00, schrieb W. Trevor King: > > From: "W. Trevor King" > > > > This allows users to override the .gitmodules value with a > > per-repository value. > > Your intentions makes lots of sense, but your patch does more than > that. Copying the branch setting into .git/config sets the initial > branch setting into stone. That makes it impossible to have a branch > "foo" in the superproject using a branch "bar" in a submodule and > another superproject branch "frotz" using branch "nitfol" for the > same submodule. You should use the branch setting from .git/config > if present and fall back to the branch setting from .gitmodules if > not, which would enable the user to have her own setting if she > doesn't like what upstream provides but would still enable others > to follow different submodule branches in different superproject > branches. I've mulling this over, and when I started coding support for submodule..remote, I had an idea. On Thu, Nov 29, 2012 at 10:27:19PM -0500, W. Trevor King wrote: > On Thu, Nov 29, 2012 at 08:11:20PM -0500, Phil Hord wrote: > > I've always felt that the "origin" defaults are broken and are simply > > being ignored because most users do not trip over them. But ISTR that > > submodule commands use the remote indicated by the superproject's > > current remote-tracking configuration, with a fallback to 'origin' if > > there is none. Sort of a "best effort" algorithm, I think. Am I > > remembering that wrong? > > The current code uses a bare "git-fetch". I'm not sure what that > defaults to if you're on a detached head. If it bothers you, I'm fine > adding the submodule..remote option in v6. In my v5 patch, I check for submodule..remote first in the usual `git config` files. If I don't find what I'm looking for I fall back on .gitmodules (basically Jens' suggestion). However, my initial copying-to-.git/config approach was mostly done to mimic existing configuration handling in git-submodule.sh. Since I agree with Jens on configuration precendence, and I now had two options to read (.branch and .remote), I thought I'd pull the logic out into its own function (code included at the end). While I was shifting the existing submodule config handling over to my new function, I noticed that with this logic, `submodule init` doesn't really do anything important anymore. Likewise for `submodule sync`, which seems to be quite similar to `init`. What to do about this? `init` has been around for a while, so we can't just remove it (maybe in 2.0?). Leaving it in place is not really a problem though, it just means that the user is locking in the current .gitmodules configuration (as Jens pointed out with respect to .branch). I may be way off base here, as I'm fairly new to submodules in general and these two commands in particular, but I thought I'd float the idea. Cheers, Trevor --- # # Print a submodule configuration setting # # $1 = submodule name # $2 = option name # $3 = default value # # Checks in the usual git-config places first (for overrides), # otherwise it falls back on .gitmodules. This allows you to # distribute project-wide defaults in .gitmodules, while still # customizing individual repositories if necessary. If the option is # not in .gitmodules either, print a default value. # get_submodule_config() { name="$1" option="$2" default="$3" value=$(git config submodule."$name"."$option") if test -z "$value" then value=$(git config -f .gitmodules submodule."$name"."$option") fi printf '%s' "${value:-$default}" } -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] reset: learn to reset to tree
On Thu, Nov 29, 2012 at 2:00 PM, Martin von Zweigbergk wrote: > Slightly off topic, but another difference (or somehow another aspect > of the same difference?) that has tripped me up a few times is that > "git checkout $rev ." only affects added and modified files (in $rev > compared to HEAD), but "git reset $rev ." would also delete deleted > files from the index. Actually, what is the reasoning behind this difference? It almost seems like a bug. I think I have just thought it was too obvious to be a bug before, but thinking more about it, I can't see any reason why "git checkout $rev" should delete files, but "git checkout $rev ." shouldn't. I hope I'm just hallucinating or missing something. Can someone shed some light on this? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Operations on unborn branch
On Tue, Nov 27, 2012 at 11:12 PM, Junio C Hamano wrote: > > You have to special case the edges whichever way you go. [...] If I understand you correctly, you're saying that revision walking would need a different special case. This is the most obvious difference, it seems. "git show" would also need different special-casing. But rebase wouldn't need --root, diff-tree wouldn't need --root, any operations on an unborn branch would just work (incl reset, cherry-pick). Again, this is hypothetical, so I'll stop the complaining now :-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] Can we ignore case for commiters name in shortlog?
On 30 November 2012 16:49, Max Horn wrote: > I don't see how wrong case is different from any other form of misspelling. > And mailmap is there precisely to handle such problems. Now, if these case > issues were for some reasons very frequent, it might be worth adding > dedicated support for it. But this seems dubious to me -- do you have any > evidence for this? Indeed, do you have more than just the one example? I don't have another example, but i have seen it many times. This happens when people use different repo's to send patches and by mistake have mentioned names in different case in them in their local .git/config files. -- viresh -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] wrap_in_html(): process message in bulk rather than line-by-line
On 11/29/2012 10:33 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> Now that we can xml-quote an arbitrary string in O(N), there is no >> reason to process the message line by line. This change saves lots of >> memory allocations and copying. >> >> The old code would have created invalid output for a malformed input >> message (one that does not contain a blank line separating the header >> from the body). The new code die()s in this situation. > > Given that imap-send is about sending a patch the distinction would > not matter in practice, but isn't the difference between the two > that the new version would not allow sending a header-only message > without a body, while the old one allowed it? I was thinking that the end-of-header line is a required part of an RFC2282 email message, but I was wrong. If you squash the attached patch onto this commit, it will handle emails without bodies correctly. Nevertheless, the old code was even *more* broken because it added a "" regardless of whether the separator line had been seen, and therefore a message without an end-of-header line would come out like Header1: foo Header2: bar with no content_type line, no pre_open, and appended to the header without a blank line in between. This is the "invalid output" that I was referring to. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ diff --git a/imap-send.c b/imap-send.c index eec9e35..e521e2f 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1348,7 +1348,7 @@ static void wrap_in_html(struct strbuf *msg) const char *body = strstr(msg->buf, "\n\n"); if (!body) - die("malformed message"); + return; /* Headers but no body; no wrapping needed */ body += 2;
Re: [PATCH 6/8] imap-send: change msg_data from storing (char *, len) to storing strbuf
On 11/29/2012 10:30 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> struct msg_data stored (char *, len) of the data to be included in a > > That (, ) is a bit funny notation, even though it is > understandable. I understand that it is funny, but it seems like the clearest way to express what is meant in a way that fits in the summary line. Feel free to change it if you like. >> message, kept the character data NUL-terminated, etc., much like a >> strbuf would do. So change it to use a struct strbuf. This makes the >> code clearer and reduces copying a little bit. >> >> A side effect of this change is that the memory for each message is >> freed after it is used rather than leaked, though that detail is >> unimportant given that imap-send is a top-level command. >> >> -- > > ? If by "?" you are wondering where the memory leak was, it was: * The while loop in main() called split_msg() * split_msg() cleared the msg_data structure using memset(msg, 0, sizeof *msg) * split_msg() copied the first message out of all_msgs using xmemdupz() and stored the result to msg->data * The msg_data was passed to imap_store_msg(). Its contents were copied to cb.data (which will be freed in the imap functions) but the original was left unfreed. * The next time through the loop, split_msg() zeroed the msg_data structure again, thus discarding the pointer to the xmemdupz()ed memory. The leak caused more memory than necessary to be allocated (worst case: nearly the total size of all_msgs). But (a) all_msgs is already stored in memory, so the wastage is at most a factor of 2; and (b) this all happens in main() shortly before program exit erases all sins. I didn't bother documenting this in the commit message because the patch changes the code anyway, but feel free to add the above explanation to the commit message if you think it is useful. >> For some reason, there is a bunch of infrastructure in this file for >> dealing with IMAP flags, although there is nothing in the code that >> actually allows any flags to be set. If there is no plan to add >> support for flags in the future, a bunch of code could be ripped out >> and "struct msg_data" could be completely replaced with strbuf. > > Yeah, after all these years we have kept the unused flags field > there and nobody needed anything out of it. I am OK with a removal > if it is done at the very end of the series. I don't think the removal of flags needs to be part of the same series. I suggest a separate patch series dedicated to deleting *all* the extra imap infrastructure at once. That being said, I'm not committing to do so. (We could add it to an "straightforward projects for aspiring git developers" list, if we had such a thing.) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Typo in Documentation/RelNotes/1.8.2.txt
From: "Horst H. von Brand" Signed-off-by: Horst H. von Brand --- Documentation/RelNotes/1.8.1.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/RelNotes/1.8.1.txt b/Documentation/RelNotes/1.8.1.txt index 8f53af3..ce5b5ec 100644 --- a/Documentation/RelNotes/1.8.1.txt +++ b/Documentation/RelNotes/1.8.1.txt @@ -79,7 +79,7 @@ UI, Workflows & Features case and removes the submodule working tree when it is safe. * "git send-email" used to prompt for the sender address, even when - the committer identify is well specified (e.g. via user.name and + the committer identity is well specified (e.g. via user.name and user.email configuration variables). The command no longer gives this prompt when not necessary. -- 1.8.0.1.347.gf94c325 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
A typo in RelNotes/1.8.2 (v1.8.0.1-347-gf94c325)
-- Dr. Horst H. von Brand User #22616 counter.li.org Departamento de InformaticaFono: +56 32 2654431 Universidad Tecnica Federico Santa Maria +56 32 2654239 Casilla 110-V, Valparaiso, Chile 234 Fax: +56 32 2797513 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] Can we ignore case for commiters name in shortlog?
viresh kumar writes: > I was just thinking if we can ignore case for commiter name while > listing stuff here? > So, that we get over any manual mistakes from commiter. See git-shortlog(1), section Mapping Authors. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] Can we ignore case for commiters name in shortlog?
David Aguilar wrote in message : > There's a feature that does exactly this. > http://www.kernel.org/pub/software/scm/git/docs/git-shortlog.html By the way, the mailmap ignore case which is annoying. I have commits as damien.olivier.robert+...@gmail.com and a dummy email address robert@numenor.night-elves. I thought that putting: Damien Robert in the .mailmap would unify the two adresses, but it does not: git shortlog -se 15 Damien Robert 266 Damien Robert as you can see, the Damien.Olivier.Robert+git as been lowercased to damien.olivier.robert, so I am forced to write a mailmap like this: Damien Robert Damien Robert git shortlog -se 281 Damien Robert -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] t4041 (diff-submodule-option): modernize style
- Enclose tests in single quotes as opposed to double quotes. This is the prevalent style in other tests. - Remove the unused variable $head4_full. - Indent the expected output so that it lines up with the rest of the test text. Signed-off-by: Ramkumar Ramachandra --- t/t4041-diff-submodule-option.sh | 459 +++--- 1 files changed, 229 insertions(+), 230 deletions(-) diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index d745197..2704df2 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh @@ -34,41 +34,41 @@ add_file . foo >/dev/null head1=$(add_file sm1 foo1 foo2) fullhead1=$(cd sm1; git rev-parse --verify HEAD) -test_expect_success 'added submodule' " +test_expect_success 'added submodule' ' git add sm1 && git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && -Submodule sm1 000...$head1 (new submodule) -EOF + Submodule sm1 000...$head1 (new submodule) + EOF test_cmp expected actual -" +' -test_expect_success 'added submodule, set diff.submodule' " +test_expect_success 'added submodule, set diff.submodule' ' git config diff.submodule log && git add sm1 && git diff --cached >actual && cat >expected <<-EOF && -Submodule sm1 000...$head1 (new submodule) -EOF + Submodule sm1 000...$head1 (new submodule) + EOF git config --unset diff.submodule && test_cmp expected actual -" +' -test_expect_success '--submodule=short overrides diff.submodule' " +test_expect_success '--submodule=short overrides diff.submodule' ' test_config diff.submodule log && git add sm1 && git diff --submodule=short --cached >actual && cat >expected <<-EOF && -diff --git a/sm1 b/sm1 -new file mode 16 -index 000..$head1 /dev/null -+++ b/sm1 -@@ -0,0 +1 @@ -+Subproject commit $fullhead1 -EOF + diff --git a/sm1 b/sm1 + new file mode 16 + index 000..$head1 + --- /dev/null + +++ b/sm1 + @@ -0,0 +1 @@ + +Subproject commit $fullhead1 + EOF test_cmp expected actual -" +' test_expect_success 'diff.submodule does not affect plumbing' ' test_config diff.submodule log && @@ -88,47 +88,47 @@ test_expect_success 'diff.submodule does not affect plumbing' ' commit_file sm1 && head2=$(add_file sm1 foo3) -test_expect_success 'modified submodule(forward)' " +test_expect_success 'modified submodule(forward)' ' git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && -Submodule sm1 $head1..$head2: - > Add foo3 -EOF + Submodule sm1 $head1..$head2: + > Add foo3 + EOF test_cmp expected actual -" +' -test_expect_success 'modified submodule(forward)' " +test_expect_success 'modified submodule(forward)' ' git diff --submodule=log >actual && cat >expected <<-EOF && -Submodule sm1 $head1..$head2: - > Add foo3 -EOF + Submodule sm1 $head1..$head2: + > Add foo3 + EOF test_cmp expected actual -" +' -test_expect_success 'modified submodule(forward) --submodule' " +test_expect_success 'modified submodule(forward) --submodule' ' git diff --submodule >actual && cat >expected <<-EOF && -Submodule sm1 $head1..$head2: - > Add foo3 -EOF + Submodule sm1 $head1..$head2: + > Add foo3 + EOF test_cmp expected actual -" +' -test_expect_success 'modified submodule(forward) --submodule=short' " fullhead2=$(cd sm1; git rev-parse --verify HEAD) +test_expect_success 'modified submodule(forward) --submodule=short' ' git diff --submodule=short >actual && cat >expected <<-EOF && -diff --git a/sm1 b/sm1 -index $head1..$head2 16 a/sm1 -+++ b/sm1 -@@ -1 +1 @@ --Subproject commit $fullhead1 -+Subproject commit $fullhead2 -EOF + diff --git a/sm1 b/sm1 + index $head1..$head2 16 + --- a/sm1 + +++ b/sm1 + @@ -1 +1 @@ + -Subproject commit $fullhead1 + +Subproject commit $fullhead2 + EOF test_cmp expected actual -" +' commit_file sm1 && head3=$( @@ -137,29 +137,28 @@ head3=$( git rev-parse --short --verify HEAD ) -test_expect_success 'modified submodule(backward)' " +test_expect_success 'modified submodule(backward)' ' git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && -Submodule sm1 $head2..$head3 (rewind): - < Add foo3 - < Add foo2 -EOF + Submodule sm1 $head2..$head3 (rewind): + < Add foo3 + < Add foo2 + EOF test_cmp expected actual -" +' -head4=$(add_file sm1 foo4 foo5) && -head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD) -test_expect_success 'modified submodule(backward and forward)' " +head4=$(add_file sm1 foo4 foo5) +test_expect_success 'modified submodule(backward and forward)' '
[PATCH v2 3/4] t4041 (diff-submodule-option): rewrite add_file() routine
Instead of "cd there and then come back", use the "cd there in a subshell" pattern. Also fix '&&' chaining in one place. Suggested-by: Junio C Hamano Signed-off-by: Ramkumar Ramachandra --- t/t4041-diff-submodule-option.sh | 23 +++ 1 files changed, 11 insertions(+), 12 deletions(-) diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index 08d549a..d745197 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh @@ -11,18 +11,17 @@ This test tries to verify the sanity of the --submodule option of git diff. . ./test-lib.sh add_file () { - sm=$1 - shift - owd=$(pwd) - cd "$sm" - for name; do - echo "$name" > "$name" && - git add "$name" && - test_tick && - git commit -m "Add $name" - done >/dev/null - git rev-parse --short --verify HEAD - cd "$owd" + ( + cd "$1" && + shift && + for name; do + echo "$name" > "$name" && + git add "$name" && + test_tick && + git commit -m "Add $name" || exit + done >/dev/null && + git rev-parse --short --verify HEAD + ) } commit_file () { test_tick && -- 1.7.8.1.362.g5d6df.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] t4041 (diff-submodule-option): parse digests sensibly
`git rev-list --max-count=1 HEAD` is a roundabout way of saying `git rev-parse --verify HEAD`; replace a bunch of instances of the former with the latter. Also, don't unnecessarily `cut -c1-7` the rev-parse output when the `--short` option is available. Signed-off-by: Ramkumar Ramachandra --- t/t4041-diff-submodule-option.sh | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index 5377639..08d549a 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh @@ -21,7 +21,7 @@ add_file () { test_tick && git commit -m "Add $name" done >/dev/null - git rev-parse --verify HEAD | cut -c1-7 + git rev-parse --short --verify HEAD cd "$owd" } commit_file () { @@ -33,7 +33,7 @@ test_create_repo sm1 && add_file . foo >/dev/null head1=$(add_file sm1 foo1 foo2) -fullhead1=$(cd sm1; git rev-list --max-count=1 $head1) +fullhead1=$(cd sm1; git rev-parse --verify HEAD) test_expect_success 'added submodule' " git add sm1 && @@ -116,8 +116,8 @@ EOF test_cmp expected actual " -fullhead2=$(cd sm1; git rev-list --max-count=1 $head2) test_expect_success 'modified submodule(forward) --submodule=short' " +fullhead2=$(cd sm1; git rev-parse --verify HEAD) git diff --submodule=short >actual && cat >expected <<-EOF && diff --git a/sm1 b/sm1 @@ -135,7 +135,7 @@ commit_file sm1 && head3=$( cd sm1 && git reset --hard HEAD~2 >/dev/null && - git rev-parse --verify HEAD | cut -c1-7 + git rev-parse --short --verify HEAD ) test_expect_success 'modified submodule(backward)' " @@ -220,8 +220,8 @@ EOF rm -f sm1 && test_create_repo sm1 && head6=$(add_file sm1 foo6 foo7) -fullhead6=$(cd sm1; git rev-list --max-count=1 $head6) test_expect_success 'nonexistent commit' " +fullhead6=$(cd sm1; git rev-parse --verify HEAD) git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && Submodule sm1 $head4...$head6 (commits not present) @@ -318,8 +318,8 @@ EOF " (cd sm1; git commit -mchange foo6 >/dev/null) && -head8=$(cd sm1; git rev-parse --verify HEAD | cut -c1-7) && test_expect_success 'submodule is modified' " +head8=$(cd sm1; git rev-parse --short --verify HEAD) && git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && Submodule sm1 $head6..$head8: @@ -461,7 +461,7 @@ EOF test_cmp expected actual " -fullhead7=$(cd sm2; git rev-list --max-count=1 $head7) +fullhead7=$(cd sm2; git rev-parse --verify HEAD) test_expect_success 'given commit --submodule=short' " git diff-index -p --submodule=short HEAD^ >actual && -- 1.7.8.1.362.g5d6df.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] t4041 (diff-submodule-option): don't hardcode SHA-1 in expected outputs
The expected SHA-1 digests are always available in variables. Use them instead of hardcoding. Signed-off-by: Ramkumar Ramachandra --- t/t4041-diff-submodule-option.sh | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh index 57e8a9d..5377639 100755 --- a/t/t4041-diff-submodule-option.sh +++ b/t/t4041-diff-submodule-option.sh @@ -62,7 +62,7 @@ test_expect_success '--submodule=short overrides diff.submodule' " cat >expected <<-EOF && diff --git a/sm1 b/sm1 new file mode 16 -index 000..a2c4dab +index 000..$head1 --- /dev/null +++ b/sm1 @@ -0,0 +1 @@ @@ -77,7 +77,7 @@ test_expect_success 'diff.submodule does not affect plumbing' ' cat >expected <<-EOF && diff --git a/sm1 b/sm1 new file mode 16 - index 000..a2c4dab + index 000..$head1 --- /dev/null +++ b/sm1 @@ -0,0 +1 @@ @@ -173,10 +173,10 @@ mv sm1-bak sm1 test_expect_success 'typechanged submodule(submodule->blob), --cached' " git diff --submodule=log --cached >actual && cat >expected <<-EOF && -Submodule sm1 41fbea9...000 (submodule deleted) +Submodule sm1 $head4...000 (submodule deleted) diff --git a/sm1 b/sm1 new file mode 100644 -index 000..9da5fb8 +index 000..$head5 --- /dev/null +++ b/sm1 @@ -0,0 +1 @@ @@ -190,7 +190,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' " cat >expected <<-EOF && diff --git a/sm1 b/sm1 deleted file mode 100644 -index 9da5fb8..000 +index $head5..000 --- a/sm1 +++ /dev/null @@ -1 +0,0 @@ -- 1.7.8.1.362.g5d6df.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] t4041 (diff-submodule-option): minor cleanup
Hi, v1 is here: 1354005692-2809-1-git-send-email-artag...@gmail.com This is in response to Junio's review of v1. Thanks. Ram Ramkumar Ramachandra (4): t4041 (diff-submodule-option): don't hardcode SHA-1 in expected outputs t4041 (diff-submodule-option): parse digests sensibly t4041 (diff-submodule-option): rewrite add_file() routine t4041 (diff-submodule-option): modernize style t/t4041-diff-submodule-option.sh | 496 +++--- 1 files changed, 247 insertions(+), 249 deletions(-) -- 1.7.8.1.362.g5d6df.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] Can we ignore case for commiters name in shortlog?
On 30.11.2012, at 04:35, viresh kumar wrote: > On 30 November 2012 09:03, Nicolas Pitre wrote: > >> Have a look at the .mailmap file in the top directory of your repo. > > Repeating what i said to David in other mail: > > I have my name there :) > > I thought using names with different case is actually different then > misspelling > it. And so, everybody must not be required to update their names in mailmap > with different case. So, with same email id and same name (that may be in > different case), we can show commits together in shortlog. I don't see how wrong case is different from any other form of misspelling. And mailmap is there precisely to handle such problems. Now, if these case issues were for some reasons very frequent, it might be worth adding dedicated support for it. But this seems dubious to me -- do you have any evidence for this? Indeed, do you have more than just the one example? In a nutshell, there seem to be two options here, and I know which *I* find more appealing ;) 1) continue this discussion over several emails to design a new feature (an option, config setting, whatever) to handle your special case, then sit down and write code for it, add documentation, add test cases to test it. 2) You just add a single entry to your mailmap to solve your problem at hand. :-) Cheers, Max-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Extend runtime prefix computation
Hello Erik, On Fri, Nov 30, 2012 at 11:20:52AM +0100, Erik Faye-Lund wrote: > > +#if defined(__linux__) > > + struct stat st; > > + if (!stat("/proc/self/exe", &st)) { > > + abs_argv0 = xstrdup(real_path("/proc/self/exe")); > > + } > > +#elif defined(__APPLE__) > > + /* Mac OS X has realpath, which incidentally allocates its > > own > > +* memory, which in turn is why we do all the xstrdup's in > > the > > +* other cases. */ > > + abs_argv0 = realpath(argv0, NULL); > > +#endif > ...perhaps a "GetModuleFileName(NULL, ...)" for Windows is in place here? Agreed. However, I do not use git on Windows and don't have a Windows devel toolchain in place. So I guess this should be added in a separate patch by someone actually in need of it and in a position to develop and test it? Thanks, -- Michael Weiserscience + computing ag Senior Systems Engineer Geschaeftsstelle Duesseldorf Martinstrasse 47-55, Haus A phone: +49 211 302 708 32 D-40223 Duesseldorf fax: +49 211 302 708 50 www.science-computing.de -- Vorstandsvorsitzender/Chairman of the board of management: Gerd-Lothar Leonhart Vorstand/Board of Management: Dr. Bernd Finkbeiner, Michael Heinrichs, Dr. Arno Steitz, Dr. Ingrid Zech Vorsitzender des Aufsichtsrats/ Chairman of the Supervisory Board: Philippe Miltin Sitz/Registered Office: Tuebingen Registergericht/Registration Court: Stuttgart Registernummer/Commercial Register No.: HRB 382196 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Extend runtime prefix computation
On Tue, Nov 27, 2012 at 5:30 PM, Michael Weiser wrote: > Support determining the binaries' installation path at runtime even if > called without any path components (i.e. via search path). Implement > fallback to compiled-in prefix if determination fails or is impossible. > > Signed-off-by: Michael Weiser > --- > - Has two very minor memory leaks - function is called only once per > program execution. Do we care? Alternative: Use static buffer instead. > > exec_cmd.c | 68 ++- > 1 files changed, 53 insertions(+), 15 deletions(-) > > diff --git a/exec_cmd.c b/exec_cmd.c > index 125fa6f..d50d7f8 100644 > --- a/exec_cmd.c > +++ b/exec_cmd.c > @@ -4,28 +4,22 @@ > #define MAX_ARGS 32 > > static const char *argv_exec_path; > -static const char *argv0_path; > +static const char *argv0_path = NULL; > > const char *system_path(const char *path) > { > -#ifdef RUNTIME_PREFIX > - static const char *prefix; > -#else > static const char *prefix = PREFIX; > -#endif > struct strbuf d = STRBUF_INIT; > > if (is_absolute_path(path)) > return path; > > #ifdef RUNTIME_PREFIX > - assert(argv0_path); > - assert(is_absolute_path(argv0_path)); > - > - if (!prefix && > - !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) && > - !(prefix = strip_path_suffix(argv0_path, BINDIR)) && > - !(prefix = strip_path_suffix(argv0_path, "git"))) { > + if (!argv0_path || > + !is_absolute_path(argv0_path) || > + (!(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) && > +!(prefix = strip_path_suffix(argv0_path, BINDIR)) && > +!(prefix = strip_path_suffix(argv0_path, "git" { > prefix = PREFIX; > trace_printf("RUNTIME_PREFIX requested, " > "but prefix computation failed. " > @@ -41,20 +35,64 @@ const char *system_path(const char *path) > const char *git_extract_argv0_path(const char *argv0) > { > const char *slash; > + char *abs_argv0 = NULL; > > if (!argv0 || !*argv0) > return NULL; > slash = argv0 + strlen(argv0); > > + /* walk to the first slash from the end */ > while (argv0 <= slash && !is_dir_sep(*slash)) > slash--; > > + /* if there was a slash ... */ > if (slash >= argv0) { > - argv0_path = xstrndup(argv0, slash - argv0); > - return slash + 1; > + /* ... it's either an absolute path */ > + if (is_absolute_path(argv0)) { > + /* FIXME: memory leak here */ > + argv0_path = xstrndup(argv0, slash - argv0); > + return slash + 1; > + } > + > + /* ... or a relative path, in which case we have to make it > +* absolute first and do the whole thing again */ > + abs_argv0 = xstrdup(real_path(argv0)); > + } else { > + /* argv0 is no path at all, just a name. Resolve it into a > +* path. Unfortunately, this gets system specific. */ > +#if defined(__linux__) > + struct stat st; > + if (!stat("/proc/self/exe", &st)) { > + abs_argv0 = xstrdup(real_path("/proc/self/exe")); > + } > +#elif defined(__APPLE__) > + /* Mac OS X has realpath, which incidentally allocates its own > +* memory, which in turn is why we do all the xstrdup's in the > +* other cases. */ > + abs_argv0 = realpath(argv0, NULL); > +#endif ...perhaps a "GetModuleFileName(NULL, ...)" for Windows is in place here? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] win32: support echo for terminal-prompt
Ping? On Tue, Nov 13, 2012 at 3:04 PM, Erik Faye-Lund wrote: > We currently only support getpass, which does not echo at all, for > git_terminal_prompt on Windows. The Windows console is perfectly > capable of doing this, so let's make it so. > > This implementation tries to reuse the /dev/tty-code as much as > possible. > > The big reason that this becomes a bit hairy is that Ctrl+C needs > to be handled correctly, so we don't leak the console state to a > non-echoing setting when a user aborts. > > Windows makes this bit a little bit tricky, in that we need to > implement SIGINT for fgetc. However, I suspect that this is a good > thing to do in the first place. > > An earlier iteration was also breifly discussed here: > http://mid.gmane.org/cabpqnsaucedu4+2n63n0k_xwsxop_ifzg3geyspsbpcsvv8...@mail.gmail.com > > The series can also be found here, only with an extra patch that > makes the (interactive) testing a bit easier: > > https://github.com/kusma/git/tree/work/terminal-cleanup > > Erik Faye-Lund (5): > mingw: make fgetc raise SIGINT if apropriate > compat/terminal: factor out echo-disabling > compat/terminal: separate input and output handles > mingw: reuse tty-version of git_terminal_prompt > mingw: get rid of getpass implementation > > compat/mingw.c| 91 +++--- > compat/mingw.h| 8 +++- > compat/terminal.c | 129 > -- > 3 files changed, 169 insertions(+), 59 deletions(-) > > -- > 1.8.0.7.gbeffeda > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
Junio C Hamano writes: > That "shell-style" contradicts with what fast-import.c says, though. > It claims to grok \octal and described as C-style. As Peff mentionned, my last version is better, although still a bit incomplete. My new version documents things that _must_ be escaped, but not what _can_. I'm reluctant to try being exhaustive in the .txt documentation if there is an exhaustive comment in the code. One option would be to actually turn the comment into an official specification by moving it to the .txt file, but that goes far beyond the scope of my patch. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html