* Re: [PATCH 1/3] bash-completion: Support running when set -u is enabled
From: Shawn O. Pearce @ 2009-01-15 15:34 UTC (permalink / raw)
To: ted; +Cc: git, gitster
In-Reply-To: <1231963342-24461-1-git-send-email-ted@tedpavlic.com>
ted@tedpavlic.com wrote:
> From: Ted Pavlic <ted@tedpavlic.com>
>
> Under "set -u" semantics, it is an error to access undefined
> variables. Some user environments may enable this setting in the
> interactive shell.
>
> In any context where the completion functions access an undefined
> variable, accessing a default empty string (aka "${1-}" instead of
> "$1") is a reasonable way to code the function, as it silences
> the undefined variable error while still supplying an empty string.
>
> In this patch, functions that should always take an argument still use
> $1. Functions that have optional arguments use ${1-}.
>
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
Yup. This patch isn't as bad as everyone made it out to be.
I think we should go ahead and include it.
But usually we don't indent the commit message like you did
above; instead the message should be aligned on the left margin at
column 0. git log will indent the message during display, hence
any identation you add in the patch email just makes the message
that much harder to read from git log.
> ---
> contrib/completion/git-completion.bash | 18 +++++++++---------
> 1 files changed, 9 insertions(+), 9 deletions(-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments
From: Shawn O. Pearce @ 2009-01-15 15:35 UTC (permalink / raw)
To: ted; +Cc: git, gitster
In-Reply-To: <1231963342-24461-3-git-send-email-ted@tedpavlic.com>
ted@tedpavlic.com wrote:
> From: Ted Pavlic <ted@tedpavlic.com>
>
> Adds a few simple comments above commands that take arguments. These
> comments are meant to remind developers of potential problems that can
> occur when the script is sourced on systems with "set -u." Any
> function which "requires" arguments really ought to be called with
> explicit arguments given.
>
> Also adds a #!bash to the top of bash completions so that editing
> software can always identify that the file is of sh type.
>
> Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
Yup, this and also 2/3 look fine. But again, the message, it
shouldn't be indented.
> ---
> contrib/completion/git-completion.bash | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
--
Shawn.
^ permalink raw reply
* Re: [ANNOUNCE] tig-0.13
From: bill lam @ 2009-01-15 15:41 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: git
In-Reply-To: <20090115150841.GA23045@diku.dk>
On Thu, 15 Jan 2009, Jonas Fonseca wrote:
> Then I am puzzled why the configure script doesn't find it. Can you send
> me your config.log and the output of running configure. Maybe off-list.
I just delete everything and do a git reset --hard, ./configure does
detect ncursesw and link to it. I'm not sure my previous error was
resulted from config cache. I ldd tig and confirm indeed it linked to
the unicode version.
Sorry for the noise. m(__)m
--
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3
唐詩019 孟浩然 夏日南亭懷辛大
山光忽西落 池月漸東上 散髮乘夜涼 開軒臥閑敞 荷風送香氣 竹露滴清響
欲取鳴琴彈 恨無知音賞 感此懷故人 中宵勞夢想
^ permalink raw reply
* Re: [PATCH 1/2] Move run_hook() from builtin-commit.c into run-command.c (libgit)
From: Johannes Schindelin @ 2009-01-15 15:46 UTC (permalink / raw)
To: Stephan Beyer
Cc: git, Paolo Bonzini, Miklos Vajna, Shawn O. Pearce,
Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <1232031618-5243-1-git-send-email-s-beyer@gmx.net>
Hi,
On Thu, 15 Jan 2009, Stephan Beyer wrote:
> Stripping out a libified version seemed better to me than
> copy and paste.
Oh, definitely.
> - ret = start_command(&hook);
> - if (ret) {
> - warning("Could not spawn %s", argv[0]);
> - return ret;
> - }
> - ret = finish_command(&hook);
> - if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
> - warning("%s exited due to uncaught signal", argv[0]);
What are the side effects of replacing this with "ret =
run_command(&hook);"? This has to be discussed and defended in the commit
message.
> diff --git a/run-command.c b/run-command.c
> index c90cdc5..602fe85 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -342,3 +342,38 @@ int finish_async(struct async *async)
> #endif
> return ret;
> }
> +
> +int run_hook(const char *index_file, const char *name, ...)
> +{
> + struct child_process hook;
> + const char *argv[10], *env[2];
> + char index[PATH_MAX];
> + va_list args;
> + int i;
> +
> + va_start(args, name);
> + argv[0] = git_path("hooks/%s", name);
> + i = 0;
> + do {
> + if (++i >= ARRAY_SIZE(argv))
> + die("run_hook(): too many arguments");
> + argv[i] = va_arg(args, const char *);
> + } while (argv[i]);
> + va_end(args);
> +
> + if (access(argv[0], X_OK) < 0)
> + return 0;
> +
> + memset(&hook, 0, sizeof(hook));
> + hook.argv = argv;
> + hook.no_stdin = 1;
> + hook.stdout_to_stderr = 1;
> + if (index_file) {
> + snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> + env[0] = index;
> + env[1] = NULL;
> + hook.env = env;
> + }
> +
> + return run_command(&hook);
> +}
Lots of improvements possible (I agree; _after_ this patch):
- deuglify the loop,
- use ALLOC_GROW instead of having a fixed size argv,
- use an strbuf for the index file
- checking executability of argv[0] before filling argv,
and possibly others, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/3] bash-completion: Added comments to remind about required arguments
From: Ted Pavlic @ 2009-01-15 15:47 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20090115153543.GF10179@spearce.org>
> Yup, this and also 2/3 look fine. But again, the message, it
> shouldn't be indented.
I will re-submit with no indentation.
Sorry for all the fuss. I've learned a great deal.
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
* Re: [PATCH 2/2] api-run-command.txt: talk about run_hook()
From: Jakub Narebski @ 2009-01-15 15:49 UTC (permalink / raw)
To: Stephan Beyer
Cc: git, Paolo Bonzini, Miklos Vajna, Shawn O. Pearce,
Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <1232031618-5243-2-git-send-email-s-beyer@gmx.net>
Stephan Beyer <s-beyer@gmx.net> writes:
> +`run_hook`::
> +
> + Run a hook.
> + The first argument is a string to an index file or NULL
> + if the default index file or no index is used at all.
Errr...
The first argument is a filename path to an index file,
or NULL if hook uses default index file or no index is
used at all.
> + The second argument is the name of the hook.
O.K.
> + The further variable number (up to 9) of arguments correspond
> + to the hook arguments.
> + The last argument has to be NULL to terminate the variable
> + arguments list.
Why the limitation of maximum of 9 hook arguments?
> + If the hook is not executable, the return value is zero.
Or the hook does not exist, I assume.
> + If it is executable, the hook will be executed and the exit
> + status of the hook is returned.
> + On execution, .stdout_to_stderr and .no_stdin will be set.
> + (See below.)
> +
>
> Data structures
> ---------------
> --
> 1.6.1.160.gecdb
>
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: SZEDER Gábor @ 2009-01-15 15:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Anders Melchiorsen, git, Johannes.Schindelin
In-Reply-To: <7vfxjlxuu5.fsf@gitster.siamese.dyndns.org>
On Wed, Jan 14, 2009 at 04:43:14PM -0800, Junio C Hamano wrote:
> I've always had trouble with the instruction we give for splitting one
> commit into two using the interactive rebase in the documentation, as it
> always had a strong "Huh?" effect on me when it suddenly starts talking
> about doing a "git reset HEAD^"; I suspect your change may improve this
> situation quite a bit.
I think we might want do differentiate editing a commit (modifying
either the commit message or the patch or both) or splitting a commit.
The first is served well with the current 'edit' rebase command IMHO.
I don't really see the point of the additional 'git reset --soft
HEAD^'.
* If you want to edit the commit message only, then you are
better off with 'git commit --amend', because it preserves the
previous commit message. But with 'git reset --soft HEAD^' and
'git commit' the commit message is "lost"; you have to use 'git
commit -c ORIG_HEAD' instead, which is not that straightforward
(and we don't have completion support for it).
* If you want to modify the patch, too, then you would have to use
'git add' anyway, regardless of whether there was a 'git reset
--soft HEAD^', or not. The only benefit of that 'reset' I'm seeing
is that in that case 'git diff --cached' would show all the changes
that would be committed; without the 'reset' 'git diff --cached
HEAD^' is needed.
But I'm not sure whether that benefit would offset the confusion of
one more rebase command with just slightly different meaning.
For the second we could introduce a new rebase command like 'split',
which would do the same as 'edit' but would also perform that 'git
reset HEAD^' mentioned in the documentation automatically. Or perhaps
it could be called 'divide', since the 's' abbreviation for 'split' is
already taken by 'squash'. (Or maybe use capital 'S' for 'split'?
might be confusing...)
Regards,
Gábor
^ permalink raw reply
* 'mail' link on http://repo.or.cz/w/git.git no workee?
From: Johannes Schindelin @ 2009-01-15 15:53 UTC (permalink / raw)
To: pasky, git
Hi Pasky,
sorry to bother you between two games of Go; Could you have a look at the
'mail' link with the commit "Update 1.6.2 draft release notes"? It is not
working for me...
That is, it links to marc (not gmane?) but finds no matches...
Ciao,
Dscho
^ permalink raw reply
* [PATCH noindent 3/3] bash-completion: Added comments to remind about required arguments
From: ted @ 2009-01-15 16:02 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ted Pavlic
In-Reply-To: <1232035343-10544-2-git-send-email-ted@tedpavlic.com>
From: Ted Pavlic <ted@tedpavlic.com>
Adds a few simple comments above commands that take arguments. These
comments are meant to remind developers of potential problems that can
occur when the script is sourced on systems with "set -u." Any
function which "requires" arguments really ought to be called with
explicit arguments given.
Also adds a #!bash to the top of bash completions so that editing
software can always identify that the file is of sh type.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
This patch series is identical to the last, but the commit message has
been stripped of its indentation (by request).
contrib/completion/git-completion.bash | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 201f9a6..f8b845a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,3 +1,4 @@
+#!bash
#
# bash completion support for core Git.
#
@@ -50,6 +51,8 @@ case "$COMP_WORDBREAKS" in
*) COMP_WORDBREAKS="$COMP_WORDBREAKS:"
esac
+# __gitdir accepts 0 or 1 arguments (i.e., location)
+# returns location of .git repo
__gitdir ()
{
if [ -z "${1-}" ]; then
@@ -67,6 +70,8 @@ __gitdir ()
fi
}
+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
+# returns text to add to bash PS1 prompt (includes branch name)
__git_ps1 ()
{
local g="$(git rev-parse --git-dir 2>/dev/null)"
@@ -119,6 +124,7 @@ __git_ps1 ()
fi
}
+# __gitcomp_1 requires 2 arguments
__gitcomp_1 ()
{
local c IFS=' '$'\t'$'\n'
@@ -131,6 +137,8 @@ __gitcomp_1 ()
done
}
+# __gitcomp accepts 1, 2, 3, or 4 arguments
+# generates completion reply with compgen
__gitcomp ()
{
local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -150,6 +158,7 @@ __gitcomp ()
esac
}
+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
__git_heads ()
{
local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -168,6 +177,7 @@ __git_heads ()
done
}
+# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
__git_tags ()
{
local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -186,6 +196,7 @@ __git_tags ()
done
}
+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
__git_refs ()
{
local i is_hash=y dir="$(__gitdir "${1-}")"
@@ -218,6 +229,7 @@ __git_refs ()
done
}
+# __git_refs2 requires 1 argument (to pass to __git_refs)
__git_refs2 ()
{
local i
@@ -226,6 +238,7 @@ __git_refs2 ()
done
}
+# __git_refs_remotes requires 1 argument (to pass to ls-remote)
__git_refs_remotes ()
{
local cmd i is_hash=y
@@ -470,6 +483,7 @@ __git_aliases ()
done
}
+# __git_aliased_command requires 1 argument
__git_aliased_command ()
{
local word cmdline=$(git --git-dir="$(__gitdir)" \
@@ -482,6 +496,7 @@ __git_aliased_command ()
done
}
+# __git_find_subcommand requires 1 argument
__git_find_subcommand ()
{
local word subcommand c=1
--
1.6.1.87.g15624
^ permalink raw reply related
* [PATCH noindent 1/3] bash-completion: Support running when set -u is enabled
From: ted @ 2009-01-15 16:02 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ted Pavlic
From: Ted Pavlic <ted@tedpavlic.com>
Under "set -u" semantics, it is an error to access undefined variables.
Some user environments may enable this setting in the interactive shell.
In any context where the completion functions access an undefined
variable, accessing a default empty string (aka "${1-}" instead of "$1")
is a reasonable way to code the function, as it silences the undefined
variable error while still supplying an empty string.
In this patch, functions that should always take an argument still use
$1. Functions that have optional arguments use ${1-}.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
This patch series is identical to the last, but the commit message has
been stripped of its indentation (by request).
contrib/completion/git-completion.bash | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7b074d7..5d1515c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -52,7 +52,7 @@ esac
__gitdir ()
{
- if [ -z "$1" ]; then
+ if [ -z "${1-}" ]; then
if [ -n "$__git_dir" ]; then
echo "$__git_dir"
elif [ -d .git ]; then
@@ -111,7 +111,7 @@ __git_ps1 ()
fi
fi
- if [ -n "$1" ]; then
+ if [ -n "${1-}" ]; then
printf "$1" "${b##refs/heads/}$r"
else
printf " (%s)" "${b##refs/heads/}$r"
@@ -143,8 +143,8 @@ __gitcomp ()
;;
*)
local IFS=$'\n'
- COMPREPLY=($(compgen -P "$2" \
- -W "$(__gitcomp_1 "$1" "$4")" \
+ COMPREPLY=($(compgen -P "${2-}" \
+ -W "$(__gitcomp_1 "${1-}" "${4-}")" \
-- "$cur"))
;;
esac
@@ -152,13 +152,13 @@ __gitcomp ()
__git_heads ()
{
- local cmd i is_hash=y dir="$(__gitdir "$1")"
+ local cmd i is_hash=y dir="$(__gitdir "${1-}")"
if [ -d "$dir" ]; then
git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
refs/heads
return
fi
- for i in $(git ls-remote "$1" 2>/dev/null); do
+ for i in $(git ls-remote "${1-}" 2>/dev/null); do
case "$is_hash,$i" in
y,*) is_hash=n ;;
n,*^{}) is_hash=y ;;
@@ -170,13 +170,13 @@ __git_heads ()
__git_tags ()
{
- local cmd i is_hash=y dir="$(__gitdir "$1")"
+ local cmd i is_hash=y dir="$(__gitdir "${1-}")"
if [ -d "$dir" ]; then
git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
refs/tags
return
fi
- for i in $(git ls-remote "$1" 2>/dev/null); do
+ for i in $(git ls-remote "${1-}" 2>/dev/null); do
case "$is_hash,$i" in
y,*) is_hash=n ;;
n,*^{}) is_hash=y ;;
@@ -188,7 +188,7 @@ __git_tags ()
__git_refs ()
{
- local i is_hash=y dir="$(__gitdir "$1")"
+ local i is_hash=y dir="$(__gitdir "${1-}")"
local cur="${COMP_WORDS[COMP_CWORD]}" format refs
if [ -d "$dir" ]; then
case "$cur" in
--
1.6.1.87.g15624
^ permalink raw reply related
* [PATCH noindent 2/3] bash-completion: Try bash completions before simple filetype
From: ted @ 2009-01-15 16:02 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Ted Pavlic
In-Reply-To: <1232035343-10544-1-git-send-email-ted@tedpavlic.com>
From: Ted Pavlic <ted@tedpavlic.com>
When a git completion is not found, a bash shell should try bash-type
completions first before going to standard filetype completions. This
patch /adds/ "-o bashdefault" to the completion line. If that option is
not available, it uses the old method.
This behavior was inspired by Mercurial's bash completion script.
Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
This patch series is identical to the last, but the commit message has
been stripped of its indentation (by request).
contrib/completion/git-completion.bash | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5d1515c..201f9a6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1766,13 +1766,16 @@ _gitk ()
__git_complete_revlist
}
-complete -o default -o nospace -F _git git
-complete -o default -o nospace -F _gitk gitk
+complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
+ || complete -o default -o nospace -F _git git
+complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
+ || complete -o default -o nospace -F _gitk gitk
# The following are necessary only for Cygwin, and only are needed
# when the user has tab-completed the executable name and consequently
# included the '.exe' suffix.
#
if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o default -o nospace -F _git git.exe
+complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
+ || complete -o default -o nospace -F _git git.exe
fi
--
1.6.1.87.g15624
^ permalink raw reply related
* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:04 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <496F4BF0.6020805@drmicha.warpmail.net>
Hi,
On Thu, 15 Jan 2009, Michael J Gruber wrote:
> I'm not sure what -p is supposed to do:
>
> A) Should it preserve all merge commits which it would need to rewrite?
> That is lot to ask. Previous behaviour (intended or not) seemed to be to
> do nothing in this case where the merge connects master and work.
>
> B) Should it preserve only merges in side branches? I seem to mean by
> that branches where the parents are on work and other branches but not
> on master.
The intention was this:
$ git rebase -p master
would need to rewrite _all_ commits that are in "master..". All of them,
including the merge commits.
The fact that I implemented it as "-i -p" is only due to technical
reasons; I know (ahem, now I should put that into the past tense) the code
base pretty well.
An additional shortcut was to avoid rewriting commits when they did not
need rewriting. IOW if the commit-to-pick has only parents that were
_not_ rewritten, we can avoid cherry-picking or merging, and just reset
--hard <original commit>.
There was a problem, though; for some reason, the code as I did it fscked
up the order of the commits when -p was specified. Therefore, rewritten
commits had the wrong parents.
I thought it should be easy to fix, but then I got a job that I actually
like, so my Git time budget was tremendously reduced.
> > The more I think about it, I think it's possible I broke it with the
> > introduction of the "noop".
>
> It certainly worked after the noop introduction before the r-i-p series,
> but not any more after.
Umm... which rebase -i -p series do you mean? "noop" was introduced
pretty recently if my Alzheimered brain does not fool me.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/2] api-run-command.txt: talk about run_hook()
From: Miklos Vajna @ 2009-01-15 16:12 UTC (permalink / raw)
To: Jakub Narebski
Cc: Stephan Beyer, git, Paolo Bonzini, Shawn O. Pearce,
Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <m38wpczi09.fsf@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 469 bytes --]
On Thu, Jan 15, 2009 at 07:49:51AM -0800, Jakub Narebski <jnareb@gmail.com> wrote:
> > + The further variable number (up to 9) of arguments correspond
> > + to the hook arguments.
> > + The last argument has to be NULL to terminate the variable
> > + arguments list.
>
> Why the limitation of maximum of 9 hook arguments?
That's how it is in builtin-commit at the moment, and I agree with Dscho
about it could be a separate patch to remove this limitation.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 16:24 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.DEB.1.00.0901151658060.3586@pacific.mpi-cbg.de>
On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> The intention was this:
>
> $ git rebase -p master
>
> would need to rewrite _all_ commits that are in "master..". All of them,
> including the merge commits.
I went hog wild with all sorts of test cases and my head is
spinning, but -- even when things happen more predictably,
I'm unable to make "rebase -p" carry an evil merge over.
The "evil" part stays behind.
I'm not sure if that is intentional or not, or (more likely)
my brain has become addled and I missed something somewhere.
Regards,
Sitaram
^ permalink raw reply
* Re: [PATCH v2] checkout: implement "-" shortcut name for last branch
From: Johan Herland @ 2009-01-15 16:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Sixt, Thomas Rast, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0901151413250.3586@pacific.mpi-cbg.de>
On Thursday 15 January 2009, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > Thomas Rast schrieb:
> > > Let git-checkout save the old branch as a symref in LAST_HEAD,
> > > and make 'git checkout -' switch back to LAST_HEAD, like 'cd -'
> > > does in the shell.
> >
> > /me likes this feature.
> >
> > git rebase (-i or not) calls checkout behind the scenes if the
> > two-argument form is used:
> >
> > git rebase [-i] master topic
> >
> > and 'topic' is not the current branch. You may want to add a test
> > that ensures that rebase sets LAST_HEAD in this case.
> >
> > You must make sure that commits referenced by LAST_HEAD are not
> > garbage-collected. (I don't know if this happens anyway for symrefs
> > in .git.)
>
> Note: if you used reflogs for that feature, the garbage collection
> could not have killed the commit. However, it is quite possible that
> the branch was deleted.
I also like this feature, but as this is only a _convenience_ feature, I
would prefer if it didn't keep the previous branch/commit alive (if
otherwise unreachable). In any case, this new feature will _have_ to
handle the case where there simply is no previous branch/commit (e.g.
after a git clone or git init).
I suggest that "git checkout -" looks at the reflog, and if there is no
previous entry in the reflog, or that entry is unreachable, then fail
in the same manner as "git checkout garbage"
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:43 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Michael J Gruber, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <20090115144050.GD10045@leksak.fem-net>
Hi,
On Thu, 15 Jan 2009, Stephan Beyer wrote:
> Michael J Gruber wrote:
> > If it's about to be integrated we can do without the
> > present script...
>
> I think it will take some time and some discussions on the list until it
> will be integrated. I remember, for example, Dscho, who has, since it
> had first come up, always been opposed to the mark-reset /
> mark-reset-merge scheme (in rebase -i -p, at least). Other users said
> "Wow, this is much more flexible." ... and this is perhaps only one
> thing that can lead to some bigger discussion.
Wow, much more flexible. Except that you should not need this kind of
flexibility. If you need to do something complicated, it would be better
to use "rebase -i -p" for the parts that do _not_ need to pick _other_
parents than are recorded in the commits.
And then you do an "edit" (or "pause" or whatever), and cherry-pick/merge
_explicitely_ what you want.
Further, keep in mind that not only is that flexibility of dubitable value
to the most users, it is also confusing, _and_ it adds code that is so
rarely exercized that bugs can lurk in there for years... as you can
experience right now.
So no, nothing has changed, I find that mark idea still horrible,
horrible, horrible.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2] checkout: implement "-" shortcut name for last branch
From: Johannes Schindelin @ 2009-01-15 16:50 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Johannes Sixt, Thomas Rast, Junio C Hamano
In-Reply-To: <200901151732.57023.johan@herland.net>
Hi,
On Thu, 15 Jan 2009, Johan Herland wrote:
> On Thursday 15 January 2009, Johannes Schindelin wrote:
>
> > On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > > Thomas Rast schrieb:
> > > > Let git-checkout save the old branch as a symref in LAST_HEAD,
> > > > and make 'git checkout -' switch back to LAST_HEAD, like 'cd -'
> > > > does in the shell.
> > >
> > > /me likes this feature.
> > >
> > > git rebase (-i or not) calls checkout behind the scenes if the
> > > two-argument form is used:
> > >
> > > git rebase [-i] master topic
> > >
> > > and 'topic' is not the current branch. You may want to add a test
> > > that ensures that rebase sets LAST_HEAD in this case.
> > >
> > > You must make sure that commits referenced by LAST_HEAD are not
> > > garbage-collected. (I don't know if this happens anyway for symrefs
> > > in .git.)
> >
> > Note: if you used reflogs for that feature, the garbage collection
> > could not have killed the commit. However, it is quite possible that
> > the branch was deleted.
>
> I also like this feature, but as this is only a _convenience_ feature, I
> would prefer if it didn't keep the previous branch/commit alive (if
> otherwise unreachable).
You misread me: if the information is in HEAD's reflog, the _is_
reachable. From HEAD's reflog. And therefore, the objects will not be
gc'ed (yet).
> In any case, this new feature will _have_ to handle the case where there
> simply is no previous branch/commit (e.g. after a git clone or git
> init).
>
> I suggest that "git checkout -" looks at the reflog, and if there is no
> previous entry in the reflog, or that entry is unreachable, then fail
> in the same manner as "git checkout garbage"
Exactly my thinking.
Ciao,
Dscho
^ permalink raw reply
* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:53 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: git
In-Reply-To: <slrngmuoq8.3u2.sitaramc@sitaramc.homelinux.net>
Hi,
if you would like me to respond to your questions in the future, it is
mandatory to keep me in the Cc: list.
On Thu, 15 Jan 2009, Sitaram Chamarty wrote:
> On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > The intention was this:
> >
> > $ git rebase -p master
> >
> > would need to rewrite _all_ commits that are in "master..". All of them,
> > including the merge commits.
>
> I went hog wild with all sorts of test cases and my head is
> spinning, but -- even when things happen more predictably,
> I'm unable to make "rebase -p" carry an evil merge over.
> The "evil" part stays behind.
>
> I'm not sure if that is intentional or not, or (more likely)
> my brain has become addled and I missed something somewhere.
Yes, this is intentional.
Instead of ignoring merges, try to recreate them.
That means it tries to recreate them. Not that it is successful. And not
even that it realizes when it failed.
Hth,
Dscho
^ permalink raw reply
* Re: rebase -p confusion in 1.6.1
From: Michael J Gruber @ 2009-01-15 16:56 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <alpine.DEB.1.00.0901151658060.3586@pacific.mpi-cbg.de>
Johannes Schindelin venit, vidit, dixit 15.01.2009 17:04:
...
>>> The more I think about it, I think it's possible I broke it with the
>>> introduction of the "noop".
>> It certainly worked after the noop introduction before the r-i-p series,
>> but not any more after.
>
> Umm... which rebase -i -p series do you mean? "noop" was introduced
> pretty recently if my Alzheimered brain does not fool me.
This one introduced noop:
commit ff74126c03a8dfd04e7533573a5c420f2a7112ac
Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Fri Oct 10 13:42:12 2008 +0200
rebase -i: do not fail when there is no commit to cherry-pick
This is the bad one from bisect:
commit d80d6bc146232d81f1bb4bc58e5d89263fd228d4
Author: Stephen Haberman <stephen@exigencecorp.com>
Date: Wed Oct 15 02:44:39 2008 -0500
rebase-i-p: do not include non-first-parent commits touching UPSTREAM
It's the last one in a longer series. And that series is after the noop
introduction.
Michael
^ permalink raw reply
* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Junio C Hamano @ 2009-01-15 16:59 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Johannes Schindelin, Johan Herland, git, Johannes Sixt,
Anders Melchiorsen, gitster
In-Reply-To: <bd6139dc0901150445l51f3b861n5bbd85bb6d1382b6@mail.gmail.com>
"Sverre Rabbelier" <srabbelier@gmail.com> writes:
> On Thu, Jan 15, 2009 at 13:36, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> If at all, I'd introduce 'examine' as a synonym to 'edit' (might be more
>> intuitive).
>
> Examine suggests that you cannot change the commit (you can look, but
> don't touch it!), no?
'stop' would be closest to what it currently does. It stops and it is up
to you how to screw up the result ;-).
^ permalink raw reply
* [JGIT PATCH] Fix RawParseUtils to match on the end of the buffer
From: Shawn O. Pearce @ 2009-01-15 17:02 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
When we were matching exactly on the end of the buffer the match
method was failing because it incorrectly tested for >= on the
buffer length. We only needed to test for > the buffer length.
JUnit tests for the match function are included to test for this
condition, and others.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../spearce/jgit/util/RawParseUtils_MatchTest.java | 69 ++++++++++++++++++++
.../src/org/spearce/jgit/util/RawParseUtils.java | 4 +-
2 files changed, 71 insertions(+), 2 deletions(-)
create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java
new file mode 100644
index 0000000..32270b7
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+
+public class RawParseUtils_MatchTest extends TestCase {
+ public void testMatch_Equal() {
+ final byte[] src = Constants.encodeASCII(" differ\n");
+ final byte[] dst = Constants.encodeASCII("foo differ\n");
+ assertTrue(RawParseUtils.match(dst, 3, src) == 3 + src.length);
+ }
+
+ public void testMatch_NotEqual() {
+ final byte[] src = Constants.encodeASCII(" differ\n");
+ final byte[] dst = Constants.encodeASCII("a differ\n");
+ assertTrue(RawParseUtils.match(dst, 2, src) < 0);
+ }
+
+ public void testMatch_Prefix() {
+ final byte[] src = Constants.encodeASCII("author ");
+ final byte[] dst = Constants.encodeASCII("author A. U. Thor");
+ assertTrue(RawParseUtils.match(dst, 0, src) == src.length);
+ assertTrue(RawParseUtils.match(dst, 1, src) < 0);
+ }
+
+ public void testMatch_TooSmall() {
+ final byte[] src = Constants.encodeASCII("author ");
+ final byte[] dst = Constants.encodeASCII("author autho");
+ assertTrue(RawParseUtils.match(dst, src.length + 1, src) < 0);
+ }
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index a6a50bb..758e7af 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -71,10 +71,10 @@
* first position within b, this should match src[0].
* @param src
* the buffer to test for equality with b.
- * @return ptr += src.length if b[ptr..src.length] == src; else -1.
+ * @return ptr + src.length if b[ptr..src.length] == src; else -1.
*/
public static final int match(final byte[] b, int ptr, final byte[] src) {
- if (ptr + src.length >= b.length)
+ if (ptr + src.length > b.length)
return -1;
for (int i = 0; i < src.length; i++, ptr++)
if (b[ptr] != src[i])
--
1.6.1.233.g5b4a8
^ permalink raw reply related
* Re: [PATCH] checkout: implement "-" shortcut name for last branch
From: Thomas Rast @ 2009-01-15 17:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0901151510340.3586@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]
Johannes Schindelin wrote:
> There are a number of issues why I would like to avoid introducing
> LAST_HEAD:
>
> - it does not work when you are using different Git versions on the same
> repository,
>
> - it does not work when you switched recently,
If you switch once, you'll be able to use the feature one checkout
later than if it was reflog-based.
If you switch a lot, the feature won't be in your git half the time
anyway.
> - you are storing redundant information,
AFAIK it's the first instance of this data in a non-free-form field.
There's also the precedent of ORIG_HEAD.
> - yes, the field is meant for user consumption, but no, it is not
> free-form,
It's a field of almost arbitrary character data, filled by 70% of the
update-ref calls I can find in git.git in a "<tool>: <comment>" format
and by the rest with things such as "initial pull" or
"refs/remotes/git-svn: updating HEAD". (The latter is so informative
that it probably deserves a fix.) How is that not free-form?
> - AFAICT your version could never be convinced to resurrect deleted
> branches, without resorting to reflogs anyway.
Neither can any other use of git-checkout without the user manually
recovering some valid revspec referring to the old branch tip from the
reflog. I wanted to be able to abbreviate the previous branch's name,
and it does just that.
> - the reflog method reflects pretty much exactly how people work around
> the lack of "checkout -" currently, so why not just use the same proven
> approach?
So you can make me fight an uphill battle against your idea how it
should be done.
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Sverre Rabbelier @ 2009-01-15 17:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johan Herland, git, Johannes Sixt,
Anders Melchiorsen
In-Reply-To: <7vmydsv72u.fsf@gitster.siamese.dyndns.org>
On Thu, Jan 15, 2009 at 17:59, Junio C Hamano <gitster@pobox.com> wrote:
> 'stop' would be closest to what it currently does. It stops and it is up
> to you how to screw up the result ;-).
Hmmm, yes, I think that would be a better alias; but I think I like
the idea to wait for changes like this till sequencer goes in, mhh?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: current git kernel has strange problems during bisect
From: Andreas Bombe @ 2009-01-15 16:54 UTC (permalink / raw)
To: Kyle Moffett
Cc: Christian Borntraeger, Linus Torvalds, Sam Ravnborg,
Johannes Schindelin, git, Linux Kernel Mailing List
In-Reply-To: <f73f7ab80901131226s6af7730cucf9c44bc2b4f9545@mail.gmail.com>
On Tue, Jan 13, 2009 at 03:26:09PM -0500, Kyle Moffett wrote:
> On Sun, Jan 11, 2009 at 4:39 PM, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
> > In my opinion we should really avoid subtree merges in the future as a curtesy
> > to people who do the uncool work of testing, problem tracking and bisecting.
> > </rant>
>
> As an alternative, you can relatively easily rewrite the following
> independent histories:
>
> A -- B -- C
> X -- Y -- Z
>
> To look like this:
>
> A -- B -- C -- X' -- Y' -- Z'
>
> Where X' is (C + sub/dir/X), Y' is (C + sub/dir/Y), etc...
Given that the subtree may have been in development for a long time, it
is almost a certainty that the older commits may compile on A but not
on C. By basing it all on C you create a lot of uncompilable commits
which hurt bisection just as bad. At least with missing kernel sources
it is obvious that an attempt at compilation is futile and a waste of
time.
^ permalink raw reply
* Re: [RFC/PATCH 3/7] replace_object: add mechanism to replace objects found in "refs/replace/"
From: Christian Couder @ 2009-01-15 17:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <200901151044.45967.chriscool@tuxfamily.org>
Le jeudi 15 janvier 2009, Christian Couder a écrit :
> Le mardi 13 janvier 2009, Junio C Hamano a écrit :
>
> > Since your paradigm is prepare replacement once at the beginning, never
> > allowing to update it, I think you can update the table while you look
> > it up. E.g. if A->B and B->C exists, and A is asked for, you find A->B
> > (to tentatively make cur to point at B) and then you find B->C, and
> > before returning you can rewrite the first mapping to A->C. Later
> > look-up won't need to dereference the table twice that way.
> >
> > This assumes that there will be small number of replacements, but the
> > same object can be asked for more than once during the process.
>
> If we allow different sets of replace refs, for example A->B
> in "refs/replace/" and B->C in "refs/replace/bisect/", then we cannot
> rewrite as you suggest. We could add A->C in "refs/replace/bisect/", so
> that it overcomes A->B and B->C when we bisect, but we would not gain
> much.
Sorry, I just realized I misunderstood what you said. I don't know why but I
thought you talked about updating the refs. But in fact you are right it
should be possible to update the "replace_object" table.
Regards,
Christian.
^ 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