* [PATCH] Implement "git stash branch <newbranch> <stash>"
From: Abhijit Menon-Sen @ 2008-07-03 2:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vprpw80bw.fsf@gitster.siamese.dyndns.org>
Restores the stashed state on a new branch rooted at the commit on which
the stash was originally created, so that conflicts caused by subsequent
changes on the original branch can be dealt with.
(Thanks to Junio for this nice idea.)
---
Documentation/git-stash.txt | 19 ++++++++++++++++++-
git-stash.sh | 21 +++++++++++++++++++++
2 files changed, 39 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 23ac331..cfc1c28 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -8,8 +8,11 @@ git-stash - Stash the changes in a dirty working directory away
SYNOPSIS
--------
[verse]
-'git stash' (list | show [<stash>] | apply [<stash>] | clear | drop [<stash>] | pop [<stash>])
+'git stash' list
+'git stash' (show | apply | drop | pop ) [<stash>]
+'git stash' branch <branchname> [<stash>]
'git stash' [save [<message>]]
+'git stash' clear
DESCRIPTION
-----------
@@ -81,6 +84,20 @@ tree's changes, but also the index's ones. However, this can fail, when you
have conflicts (which are stored in the index, where you therefore can no
longer apply the changes as they were originally).
+branch <branchname> [<stash>]::
+
+ Creates and checks out a new branch named `<branchname>` starting from
+ the commit at which the `<stash>` was originally created, applies the
+ changes recorded in `<stash>` to the new working tree, and drops the
+ `<stash>` if that completes successfully. When no `<stash>` is given,
+ applies the latest one.
++
+This is useful if the branch on which you ran `git stash save` has
+changed enough that `git stash apply` fails due to conflicts. Since
+the stash is applied on top of the commit that was HEAD at the time
+`git stash` was run, it restores the originally stashed state with
+no conflicts.
+
clear::
Remove all the stashed states. Note that those states will then
be subject to pruning, and may be difficult or impossible to recover.
diff --git a/git-stash.sh b/git-stash.sh
index 4938ade..8e50b03 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -218,6 +218,23 @@ drop_stash () {
git rev-parse --verify "$ref_stash@{0}" > /dev/null 2>&1 || clear_stash
}
+apply_to_branch () {
+ have_stash || die 'Nothing to apply'
+
+ test -n "$1" || die 'No branch name specified'
+ branch=$1
+
+ if test -z "$2"
+ then
+ set x "$ref_stash@{0}"
+ fi
+ stash=$2
+
+ git-checkout -b $branch $stash^ &&
+ apply_stash $stash &&
+ drop_stash $stash
+}
+
# Main command set
case "$1" in
list)
@@ -264,6 +281,10 @@ pop)
drop_stash "$@"
fi
;;
+branch)
+ shift
+ apply_to_branch "$@"
+ ;;
*)
if test $# -eq 0
then
--
1.5.6
^ permalink raw reply related
* [PATCH] Fix describe --tags --long so it does not segfault
From: Shawn O. Pearce @ 2008-07-03 2:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Mikael Magnusson, Mark Burton
In-Reply-To: <237967ef0807021256j3e67bceaoecbb8f37112db2ab@mail.gmail.com>
If we match a lightweight (non-annotated tag) as the name to
output and --long was requested we do not have a tag, nor do
we have a tagged object to display. Instead we must use the
object we were passed as input for the long format display.
Reported-by: Mark Burton <markb@ordern.com>
Backtraced-by: Mikael Magnusson <mikachu@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Thanks Mikael, the backtrace really made it easy to figure out
what the breakage was here.
builtin-describe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-describe.c b/builtin-describe.c
index 3da99c1..e515f9c 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -204,7 +204,7 @@ static void describe(const char *arg, int last_one)
*/
display_name(n);
if (longformat)
- show_suffix(0, n->tag->tagged->sha1);
+ show_suffix(0, n->tag ? n->tag->tagged->sha1 : sha1);
printf("\n");
return;
}
--
1.5.6.74.g8a5e
^ permalink raw reply related
* Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype
From: Junio C Hamano @ 2008-07-03 1:45 UTC (permalink / raw)
To: Stephan Beyer; +Cc: git, Johannes Schindelin, Daniel Barkalow, Christian Couder
In-Reply-To: <1214879914-17866-2-git-send-email-s-beyer@gmx.net>
Stephan Beyer <s-beyer@gmx.net> writes:
> git sequencer is planned as a backend for user scripts
> that execute a sequence of git instructions and perhaps
> need manual intervention, for example git-rebase or git-am.
...
> +output () {
> + case "$VERBOSE" in
> + 0)
> + "$@" >/dev/null
> + ;;
> + 1)
> + output=$("$@" 2>&1 )
> + status=$?
> + test $status -ne 0 && printf '%s\n' "$output"
> + return $status
> + ;;
> + 2)
> + "$@"
> + ;;
> + esac
> +}
Perhaps misnamed? This feels more like "do" or "perform" or "run".
> +require_clean_work_tree () {
> + # test if working tree is dirty
> + git rev-parse --verify HEAD >/dev/null &&
> + git update-index --ignore-submodules --refresh &&
> + git diff-files --quiet --ignore-submodules &&
> + git diff-index --cached --quiet HEAD --ignore-submodules -- ||
> + die 'Working tree is dirty'
> +}
When is it necessary to ignore submodules and why? Are there cases where
submodules should not be ignored?
> +LAST_COUNT=
> +mark_action_done () {
> + sed -e 1q <"$TODO" >>"$DONE"
> + sed -e 1d <"$TODO" >"$TODO.new"
> + mv -f "$TODO.new" "$TODO"
> + if test "$VERBOSE" -gt 0
> + then
> + count=$(grep -c '^[^#]' <"$DONE")
> + total=$(expr "$count" + "$(grep -c '^[^#]' <"$TODO")")
Here we are not counting lines that are comments as insns (I am not
complaining; just making a mental note).
> + if test "$LAST_COUNT" != "$count"
> + then
> + LAST_COUNT="$count"
> + test "$VERBOSE" -lt 1 ||
> + printf 'Sequencing (%d/%d)\r' "$count" "$total"
> + test "$VERBOSE" -lt 2 || echo
> + fi
> + fi
> +}
> +
> +# Generate message, patch and author script files
> +make_patch () {
> + parent_sha1=$(git rev-parse --verify "$1"^) ||
> + die "Cannot get patch for $1^"
> + git diff-tree -p "$parent_sha1..$1" >"$PATCH"
Could there be a case where we need/want to deal with a root commit
without parents?
> + test -f "$MSG" ||
> + commit_message "$1" >"$MSG"
> + test -f "$AUTHOR_SCRIPT" ||
> + get_author_ident_from_commit "$1" >"$AUTHOR_SCRIPT"
> +}
> +
> +# Generate a patch and die with "conflict" status code
> +die_with_patch () {
> + make_patch "$1"
> + git rerere
> + die_to_continue "$2"
> +}
> +
> +restore () {
> + git rerere clear
> +
> + HEADNAME=$(cat "$SEQ_DIR/head-name")
> + HEAD=$(cat "$SEQ_DIR/head")
Perhaps
read HEADNAME <"$SEQ_DIR/head-name"
provided if these values are $IFS safe?
> + case $HEADNAME in
> + refs/*)
> + git symbolic-ref HEAD "$HEADNAME"
> + ;;
> + esac &&
> + output git reset --hard "$HEAD"
> +}
> +
> +has_action () {
> + grep '^[^#]' "$1" >/dev/null
> +}
> +
> +# Check if text file $1 contains a commit message
> +has_message () {
> + test -n "$(sed -n -e '/^Signed-off-by:/d;/^[^#]/p' <"$1")"
> +}
Makes one wonder if we would want to special case other kinds like
"Acked-by:" as well...
> +# Usage: pick_one (cherry-pick|revert) [-*|--edit] sha1
> +pick_one () {
> + what="$1"
> + # we just assume that this is either cherry-pick or revert
> + shift
> +
> + # check for fast-forward if no options are given
> + if expr "x$1" : 'x[^-]' >/dev/null
> + then
> + test "$(git rev-parse --verify "$1^")" = \
> + "$(git rev-parse --verify HEAD)" &&
> + output git reset --hard "$1" &&
> + return
> + fi
> + test "$1" != '--edit' -a "$what" = 'revert' &&
> + what='revert --no-edit'
This looks somewhat wrong.
When the history looks like ---A---B and we are at A, cherry-picking B can
be optimized to just advancing to B, but that optimization has a slight
difference (or two) in the semantics.
(1) The committer information would not record the user and time of the
sequencer operation, which actually may be a good thing.
(2) When $what is revert, this codepath shouldn't be exercised, should it?
(3) If B is a merge, even if $what is pick, this codepath shouldn't be
exercised, should it?
As to the syntax I tend to prefer
case "$1" in
-*) ... do option thing ... ;;
*) ... do other thing... ;;
esac
So how about...
case "$what,$1" in
revert,--edit)
what='revert --no-edit' ;;
revert,* | cherry-pick,-* )
;;
*)
if ! git rev-parse --verify "$1^2" &&
test "$(git rev-parse --verify "$1^") = \
"$(git rev-parse --verify HEAD)"
then
output git reset --hard "$1"
return
fi
;;
esac
> +make_squash_message () {
> + if test -f "$squash_msg"
> + then
> + count=$(($(sed -n -e 's/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p' \
> + <"$squash_msg" | sed -n -e '$p')+1))
> + echo "# This is a combination of $count commits."
> + sed -e '1d' -e '2,/^./{
> + /^$/d
> + }' <"$squash_msg"
> + else
> + count=2
> + echo '# This is a combination of 2 commits.'
> + echo '# The first commit message is:'
> + echo
> + commit_message HEAD
> + fi
> + echo
> + echo "# This is the $(nth_string "$count") commit message:"
> + echo
> + commit_message "$1"
> +}
> +
> +make_squash_message_multiple () {
> + echo '# This is a dummy to get the 0.' >"$squash_msg"
> + for cur_sha1 in $(git rev-list --reverse "$sha1..HEAD")
> + do
> + make_squash_message "$cur_sha1" >"$MSG"
> + cp "$MSG" "$squash_msg"
> + done
> +}
Hmm, I know this is how rebase-i is written, but we should be able to do
better than writing and flipping temporary times N times, shouldn't we?
> +peek_next_command () {
> + sed -n -e '1s/ .*$//p' <"$TODO"
> +}
... which could respond "the next command is '#' (comment)", so we are
actively counting a comment as a step here. Does this contradict with the
mental note we made earlier, and if so, does the discrepancy hurt us
somewhere in this program?
> +# If $1 is a mark, make a ref from it; otherwise keep it
> +mark_to_ref () {
> + arg="$1"
> + ref=$(expr "x$arg" : 'x:0*\([0-9][0-9]*\)$')
You might want to leave comments to describe constraints that led to this
slightly awkward regexp:
* :0 is allowed
* :01 is the same as :1
> +strategy_check () {
> + case "$1" in
> + resolve|recursive|octopus|ours|subtree|theirs)
> + return
> + ;;
> + esac
> + todo_warn "Strategy '$1' not known."
> +}
Hmm. Do we need to maintain list of available strategies here and then in
git-merge separately?
> +### Author script functions
> +
> +clean_author_script () {
> + cat "$ORIG_AUTHOR_SCRIPT" >"$AUTHOR_SCRIPT"
> +}
> +
> +# Take "Name <e-mail>" in stdin and outputs author script
> +make_author_script_from_string () {
> + sed -e 's/^\(.*\) <\(.*\)>.*$/GIT_AUTHOR_NAME="\1"\
> +GIT_AUTHOR_EMAIL="\2"\
> +GIT_AUTHOR_DATE=/'
> +}
If you are going to "."-source or eval the output from this, you would
need to quote the values a lot more robustly, wouldn't you? Is this safe
against shell metacharacters in names, mails and/or space between unixtime
and the timezone information?
> + if test -z "$AUTHOR"
> + then
> + sed -n -e '
> + s/^Author: \(.*\)$/GIT_AUTHOR_NAME="\1"/p;
> + s/^Email: \(.*\)$/GIT_AUTHOR_EMAIL="\1"/p;
> + s/^Date: \(.*\)$/GIT_AUTHOR_DATE="\1"/p
> + ' <"$infofile" >>"$AUTHOR_SCRIPT"
The same comment on quoting applies here, I think.
> + # If sed's result is empty, we keep the original
> + # author script by appending.
> + fi
> ...
> + failed=
> + with_author git apply $apply_opts --index "$PATCH" || failed=t
> +
> + if test -n "$failed" -a -n "$threeway" && (with_author fallback_3way)
> + then
> + # Applying the patch to an earlier tree and merging the
> + # result may have produced the same tree as ours.
> + git diff-index --quiet --cached HEAD -- && {
> + echo 'No changes -- Patch already applied.'
> + return 0
> + # XXX: do we want that?
> + }
> + # clear apply_status -- we have successfully merged.
> + failed=
> + fi
> +
> + if test -n "$failed"
> + then
> + # XXX: This is just a stupid hack:
> + with_author git apply $apply_opts --reject --index "$PATCH"
Please don't do this without being asked, if you are planning to use this
in "am" when 3-way fallback was not asked. It _may_ make sense to give an
option to the users to ask for .rej if they prefer to work that way better
than working with 3-way merge fallback, but doing this without being asked
is not acceptable.
> + die_to_continue 'Patch failed. See the .rej files.'
> + # XXX: We actually needed a git-apply flag that creates
> + # conflict markers and sets the DIFF_STATUS_UNMERGED flag.
That is what -3way is all about, and this codepath is when the user did
not ask for it, isn't it?
> +# Check the "pick" instruction
> +check_pick () {
> + revert=
> + mainline=
> + while test $# -gt 1
> + do
> ...
> + done
> +
> + if test -n "$mainline"
> + then
> + test -z "$revert" ||
> + todo_error "Cannot use $revert together with --mainline."
Why not? If you have this...
---A---C---D
/
---B
and you are at D, you may want to undo the merge you made at C and go back
to either A or B, which essentially is same as cherry-picking diff between
C and D on top of either A or B. Both are valid operations aren't they?
The remainder of the review will have to be in a separate message..
^ permalink raw reply
* Re: [PATCH 12/12] [TODO] setup: bring changes from 4msysgit/next to next
From: Junio C Hamano @ 2008-07-03 6:13 UTC (permalink / raw)
To: prohaska
Cc: Johannes Sixt, msysGit, Johannes Schindelin, Git Mailing List,
Dmitry Kakurin
In-Reply-To: <B01C53C6-6FB6-44EF-987F-1574A77C2F95@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> On Jul 2, 2008, at 9:32 PM, Johannes Sixt wrote:
>
>> The setup.c in mingw.git (and soon Junio's master) and Junio's next
>> are
>> _different_, but both are correct. If you reverse-apply the patch you
>> presented here, then you get the version from Junio's next, which is
>> a good
>> state.
>
> Ok, I'll wait until Junio's master has the changes and will remove
> the changes to 4msysgit then.
There will be a slight difference between the setup.c in soon-to-be master
and j6t/mingw (14086b0), due to recent optimization already on 'master'
from Linus, 044bbbc (Make git_dir a path relative to work_tree in
setup_work_tree(), 2008-06-19).
diff --git a/setup.c b/setup.c
index 8bb7b10..cc3fb38 100644
--- a/setup.c
+++ b/setup.c
@@ -308,9 +308,10 @@ void setup_work_tree(void)
work_tree = get_git_work_tree();
git_dir = get_git_dir();
if (!is_absolute_path(git_dir))
- set_git_dir(make_absolute_path(git_dir));
+ git_dir = make_absolute_path(git_dir);
if (!work_tree || chdir(work_tree))
die("This operation must be run in a work tree");
+ set_git_dir(make_relative_path(git_dir, work_tree));
initialized = 1;
}
^ permalink raw reply related
* Re: [PATCH 12/12] [TODO] setup: bring changes from 4msysgit/next to next
From: Steffen Prohaska @ 2008-07-03 6:06 UTC (permalink / raw)
To: Johannes Sixt
Cc: msysGit, Johannes Schindelin, Git Mailing List, Junio C Hamano,
Dmitry Kakurin
In-Reply-To: <200807022132.27751.johannes.sixt@telecom.at>
On Jul 2, 2008, at 9:32 PM, Johannes Sixt wrote:
> On Mittwoch, 2. Juli 2008, Steffen Prohaska wrote:
>> On Jul 2, 2008, at 6:17 PM, Johannes Schindelin wrote:
>>> On Wed, 2 Jul 2008, Steffen Prohaska wrote:
>>>> From: Johannes Sixt <johannes.sixt@telecom.at>
>>>>
>>>> Hannes,
>>>> You introduced "minoffset" in 861429a7c37c7.
>>>
>>> AFAICT it was redone differently in 'next', because 'next' has this
>>> ceiling dir thingie, which allows a different (much smaller) patch.
>>>
>>> It might be more sensible to base your patch series on 'next'...
>>
>> Hmm.. it is based on next. But obviously I needed to merge
>> mingw's master to 4msysgit's master and resolve conflicts.
>> Maybe I made the wrong decisions then.
>>
>> Hannes,
>> If you believe that your setup.c is good, then I'll copy your version
>> to 4msysgit's master.
>
> The setup.c in mingw.git (and soon Junio's master) and Junio's next
> are
> _different_, but both are correct. If you reverse-apply the patch you
> presented here, then you get the version from Junio's next, which is
> a good
> state.
Ok, I'll wait until Junio's master has the changes and will remove
the changes to 4msysgit then.
Steffen
^ permalink raw reply
* Re: [PATCH 06/12] connect: Fix custom ports with plink (Putty's ssh)
From: Edward Z. Yang @ 2008-07-03 3:07 UTC (permalink / raw)
To: git; +Cc: gitster, msysGit, junio
Johannes Sixt wrote:
> What about installing a wrapper script, plinkssh, that does this:
> [snip]
Well, the patch is shorter :-)
Joking aside, it's a good question. I guess I prefer the patch because:
1. It's been tested, it works. I haven't tried the script yet, so I
don't know if it works.
2. Git historically doesn't use bash, so the script would have to be
rewritten in Perl or plain sh or tcl or something.
3. It's less brittle than the wrapper script if we decide to have Git
pass more params to OpenSSH.
4. It's "more native".
I don't know if these are compelling enough reasons, though.
(cc'ed everyone else, whoops)
^ permalink raw reply
* Re: [PATCH] fix typoed config option 'indexversion' in man page.
From: Mikael Magnusson @ 2008-07-03 1:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <alpine.LNX.1.00.0807030303580.30922@localhost>
Blah, ignore me of course. I broke the old rule of not sending patches
after midnight :).
2008/7/3 Mikael Magnusson <mikachu@gmail.com>:
> ---
>
> Noticed by David Parra on #git.
>
> Documentation/config.txt | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 52d01b8..fd56dac 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -945,7 +945,7 @@ pack.threads::
> Specifying 0 will cause git to auto-detect the number of CPU's
> and set the number of threads accordingly.
>
> -pack.indexVersion::
> +pack.indexversion::
> Specify the default pack index version. Valid values are 1 for
> legacy pack index used by Git versions prior to 1.5.2, and 2 for
> the new pack index with capabilities for packs larger than 4 GB
> --
> 1.5.6.GIT
>
>
--
Mikael Magnusson
^ permalink raw reply
* Re: finding deleted file names
From: Mikael Magnusson @ 2008-07-03 1:12 UTC (permalink / raw)
To: geoffrey.russell; +Cc: git
In-Reply-To: <93c3eada0807021701m13b7adddv51537f4cf9d52533@mail.gmail.com>
2008/7/3 Geoff Russell <geoffrey.russell@gmail.com>:
> git diff --diff-filter=D --name-only HEAD@{'7 days ago'}
>
> finds files deleted during the last 7 days, but if my repository is
> only 6 days old I get a
> fatal error.
>
> fatal: bad object HEAD@{7 days ago}
>
> Is there something that says "since repository creation", ie., go back as far
> as possible, but no further? Is there a symbolic name for the initial commit?
There's no symbolic name for it, since there might not be only one initial
commit. git.git for example has at least three root commits. You will
probably get what you want with $(git rev-list HEAD|tail -1). If your
history is very large, $(git rev-list --reverse HEAD|head -1) is slightly
faster, but usually not enough to offset typing --reverse :).
--
Mikael Magnusson
^ permalink raw reply
* [PATCH] fix typoed config option 'indexversion' in man page.
From: Mikael Magnusson @ 2008-07-03 1:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
---
Noticed by David Parra on #git.
Documentation/config.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 52d01b8..fd56dac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -945,7 +945,7 @@ pack.threads::
Specifying 0 will cause git to auto-detect the number of CPU's
and set the number of threads accordingly.
-pack.indexVersion::
+pack.indexversion::
Specify the default pack index version. Valid values are 1 for
legacy pack index used by Git versions prior to 1.5.2, and 2 for
the new pack index with capabilities for packs larger than 4 GB
--
1.5.6.GIT
^ permalink raw reply related
* Re: [BUG] Git looks for repository in wrong directory
From: David ‘Bombe’ Roden @ 2008-07-03 1:05 UTC (permalink / raw)
To: git
In-Reply-To: <7vtzf76c60.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 922 bytes --]
On Thursday 03 July 2008 02:31:35 Junio C Hamano wrote:
> This is age old usability feature that lets you say "ls-remote r1" even
> when you do *not* have "r1.git"
Is it just me or does that sentence not make any sense at all? ;)
I mean, _of cource_ I want the contents of "r1" if I say "give me the contents
of r1". I could understand if Git looked in "r1.git" if there was no "r1" but
the way it is currently done is plain wrong. IMHO, of course.
> If you have both, you already have found the way to disambiguate ;-)
Yes, _now_ I know. In my opinion it’s very unintuitive and should be changed.
And if—for some strange reason—this is to be kept as a compatibility feature
it should at least be documented somewhere in large red blinking letters that
under certain circumstances Git doesn’t care about the path you give it but
simply chooses to look somewhere else. :)
David
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* bug found (Re: git-fast-export SIGSEGV on solaris + backtrace)
From: SungHyun Nam @ 2008-07-03 0:59 UTC (permalink / raw)
To: git; +Cc: Pieter de Bie
In-Reply-To: <4869B91B.9000709@posdata.co.kr>
Hello,
From the gdb trace:
(gdb) p deco
$1 = (struct object_decoration *) 0x193150
(gdb) p idnums
$3 = {name = 0x0, size = 1500, nr = 7, hash = 0x190270}
It was 'off-by-one' error.
0x190270 + (1500 * 8) = 0x193150.
And the code says it:
for (i = 0; i < idnums.size; ++i) {
deco++;
if (deco && deco->base && deco->base->type == 1) {
The 'deco' should be post-incremented? or
Checking code should be (i < idnums.size - 1)?
Regards,
namsh
And very minor cleanup:
diff --git a/decorate.c b/decorate.c
index 23f6b00..d8b428c 100644
--- a/decorate.c
+++ b/decorate.c
@@ -36,7 +36,7 @@ static void *insert_decoration(struct decoration *n,
struct o>
static void grow_decoration(struct decoration *n)
{
int i;
- int old_size = n->size;
+ int old_size;
struct object_decoration *old_hash;
old_size = n->size;
SungHyun Nam wrote:
> Hello,
>
> Because recent GIT test failed on t9301-fast-export.sh, I traced
> it. And I found git-fast-export killed by SIGSEGV. I include a
> gdb backtrace below.
> If you want to me to check other things, please let me know.
>
> Regards,
> namsh
>
> [marks] ~/srcs/git/t/trash directory[66]$ gdb ../../git-fast-export
> GNU gdb 6.0
> Copyright 2003 Free Software Foundation, Inc.
> GDB is free software, covered by the GNU General Public License, and you
> are
> welcome to change it and/or distribute copies of it under certain
> conditions.
> Type "show copying" to see the conditions.
> There is absolutely no warranty for GDB. Type "show warranty" for details.
> This GDB was configured as "sparc-sun-solaris2.9"...
> (gdb) r --export-marks=tmp-marks HEAD
> Starting program: /flyvo2/home/namsh/srcs/git/git-fast-export
> --export-marks=tmp-marks HEAD
> blob
> mark :1
> data 8
> Wohlauf
>
> reset refs/heads/marks
> commit refs/heads/marks
> mark :2
> author A U Thor <author@example.com> 1112911993 -0700
> committer C O Mitter <committer@example.com> 1112911993 -0700
> data 8
> initial
> M 100644 :1 file
>
> blob
> mark :3
> data 9
> die Luft
>
> blob
> mark :4
> data 12
> geht frisch
>
> commit refs/heads/marks
> mark :5
> author A U Thor <author@example.com> 1112912053 -0700
> committer C O Mitter <committer@example.com> 1112912053 -0700
> data 7
> second
> from :2
> M 100644 :3 file
> M 100644 :4 file2
>
> blob
> mark :6
> data 4
> und
>
> commit refs/heads/marks
> mark :7
> author A U Thor <author@example.com> 1112912113 -0700
> committer C O Mitter <committer@example.com> 1112912113 -0700
> data 6
> third
> from :5
> M 100644 :6 file2
>
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00043024 in export_marks (file=0xffbff0df "tmp-marks")
> at builtin-fast-export.c:384
> 384 if (deco && deco->base && deco->base->type == 1) {
> (gdb) p deco
> $1 = (struct object_decoration *) 0x193150
> (gdb) p deco->base
> $2 = (struct object *) 0x2009
> (gdb) p *(struct object_decoration *) 0x193150
> $5 = {base = 0x2009, decoration = 0x0}
> (gdb) p idnums
> $3 = {name = 0x0, size = 1500, nr = 7, hash = 0x190270}
> (gdb) p *(struct object_decoration *) 0x190270
> $6 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x190280
> $9 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x190290
> $10 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x1902a0
> $11 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x1902b0
> $12 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x1902c0
> $13 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x193140
> $14 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x193160
> $15 = {base = 0x35323962, decoration = 0x63613534}
> (gdb)
>
^ permalink raw reply related
* Re: [BUG] Git looks for repository in wrong directory
From: Junio C Hamano @ 2008-07-03 0:31 UTC (permalink / raw)
To: David ‘Bombe’ Roden; +Cc: git
In-Reply-To: <200807030216.28921.bombe@pterodactylus.net>
David ‘Bombe’ Roden <bombe@pterodactylus.net> writes:
> git clone r1 r1.git
> cd r1
> echo b > b
> git add b
> git commit -m "b"
> cd ..
> git ls-remote r1
> git ls-remote r1/.
>
> shows that Git searches for a repository in the wrong place. I think the last
> two commands should output exactly the same but "git ls-remote r1" actually
> lists the contents of "r1.git". Is that a bug or is this (extremely
> confusing) behaviour intended?
This is age old usability feature that lets you say "ls-remote r1" even
when you do *not* have "r1.git", and is not limited to the local file
transport but also applicable when peeking a remote repository over the
native transport.
If you have both, you already have found the way to disambiguate ;-)
^ permalink raw reply
* Re: RFC: grafts generalised
From: Junio C Hamano @ 2008-07-03 0:28 UTC (permalink / raw)
To: Petr Baudis; +Cc: Stephen R. van den Berg, git
In-Reply-To: <20080703001614.GG12567@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> On Thu, Jul 03, 2008 at 02:13:31AM +0200, Petr Baudis wrote:
>> So, the real solution is to take the commit objects you want to
>> modify, create new commit objects, then graft the new commit on all the
>> old commit children. It fits neatly in the Git philosophy, there is no
>> need at all to tweak the current infrastructure for this and it should
>> be trivial to automate, too.
>
> Oops, sorry; I stopped reading the branch of the thread I thought was
> going off on a different tangent one post too early. :-)
What you wrote was a very good summary of what Dmitry suggested earlier
;-)
^ permalink raw reply
* Re: RFC: grafts generalised
From: Petr Baudis @ 2008-07-03 0:21 UTC (permalink / raw)
To: Stephen R. van den Berg; +Cc: Jakub Narebski, git
In-Reply-To: <20080702173203.GA16235@cuci.nl>
On Wed, Jul 02, 2008 at 07:32:03PM +0200, Stephen R. van den Berg wrote:
> Also, the graft mechanism specifically is intended as a temporary
> solution until one uses filter-branch to "finalise" the result into a
> proper repository which becomes cloneable.
Grafts are _much_ older than filter-branch and I'm not sure where did
you get this idea; do we claim that in any documentation?
> >The fact that git-filter-branch (and earlier cg-admin-rewrite-hist)
> >respects grafts, and rewrites history so that grafts are no-op and are
> >not needed further is a bit of side-effect.
>
> I beg to differ. It's not a side effect, it's the proper way to get
> rid of the grafts file. Grafts are temporary and ugly. In proper
> repositories they are a sign of transition to a proper state.
> The proper state is attained by using git filter-branch.
There's nothing ugly or necessarily temporary about grafts. One example
of completely valid usage is adding previous history of a project to it
later.
First, you don't need to carry around all the archived baggage you are
probably rarely going to access anyway if you don't need to; changing a
VCS is ideal cutoff point.
Second, you don't need to worry about doing perfect conversion at the
moment of the switch.
Third, even if you think you have done it perfectly, it will turn out
later that something is wrong anyway.
Fourth, it may not be actually _clear_ what the canonical history should
be. Consider linux-kernel, you can graft the BitKeeper history (or one
of possible candidates for the ideal conversion, though one is AFAIK
clearly favoured), or you could also graft commit-per-tarball history
even from the times before BitKeeper; you certainly don't want either in
the current main history DAG.
--
Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson
^ permalink raw reply
* [BUG] Git looks for repository in wrong directory
From: David ‘Bombe’ Roden @ 2008-07-03 0:16 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 904 bytes --]
Hi.
The following sequence:
mkdir r1
cd r1
git init
echo a > a
git add a
git commit -m "a"
cd ..
git clone r1 r1.git
cd r1
echo b > b
git add b
git commit -m "b"
cd ..
git ls-remote r1
git ls-remote r1/.
shows that Git searches for a repository in the wrong place. I think the last
two commands should output exactly the same but "git ls-remote r1" actually
lists the contents of "r1.git". Is that a bug or is this (extremely
confusing) behaviour intended?
This also afflicts the behaviour of "git-pull" and friends. I cloned a
directory and tried to pull new commits but I repeatedly got stuck with an
older commit. I have to move a second directory that was named like the first
directory, only with an appended ".git", out of the way so that I could
access the repository I asked for.
Used Git version is 1.5.6.1 on Linux 2.6.25.6 (Gentoo/x86).
David
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: RFC: grafts generalised
From: Petr Baudis @ 2008-07-03 0:16 UTC (permalink / raw)
To: Stephen R. van den Berg; +Cc: git
In-Reply-To: <20080703001331.GF12567@machine.or.cz>
On Thu, Jul 03, 2008 at 02:13:31AM +0200, Petr Baudis wrote:
> So, the real solution is to take the commit objects you want to
> modify, create new commit objects, then graft the new commit on all the
> old commit children. It fits neatly in the Git philosophy, there is no
> need at all to tweak the current infrastructure for this and it should
> be trivial to automate, too.
Oops, sorry; I stopped reading the branch of the thread I thought was
going off on a different tangent one post too early. :-)
--
Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson
^ permalink raw reply
* Re: RFC: grafts generalised
From: Petr Baudis @ 2008-07-03 0:13 UTC (permalink / raw)
To: Stephen R. van den Berg; +Cc: git
In-Reply-To: <20080702143519.GA8391@cuci.nl>
On Wed, Jul 02, 2008 at 04:35:19PM +0200, Stephen R. van den Berg wrote:
> - Extend the grafts file format to support something like the following syntax:
>
> commit eb03813cdb999f25628784bb4f07b3f4c8bfe3f6
> Parent: 7bc72e647d54c2f713160b22e2e08c39d86c7c28
> Merge: 3b3da24960a82a479b9ad64affab50226df02abe 13b8f53e8ccec3b08eeb6515e6a10a2a
> Merge: ac719ed37270558f21d89676fce97eab4469b0f1
> Tree: 32fc99814b97322174dbe97ec320cf32314959e2
> Author: Foo Bar (FooBar) <foo@bar>
> AuthorDate: Sat Jun 6 13:50:44 1998 +0000
> Commit: Foo Bar (FooBar) <foo@bar>
> CommitDate: Sat Jun 7 13:50:44 1998 +0000
> Logmessage: First line of logmessage override
> Logmessage: Second line of logmessage override
> Logmessage: Etc.
Please, don't. It adds completely unnecessary complexity and it is
_not_ grafting anymore - look the word up in a dictionary. :-)
Have a look at what you wrote above - now, Git already has a way to
store all this information, right? In the commit objects!
So, the real solution is to take the commit objects you want to
modify, create new commit objects, then graft the new commit on all the
old commit children. It fits neatly in the Git philosophy, there is no
need at all to tweak the current infrastructure for this and it should
be trivial to automate, too.
--
Petr "Pasky" Baudis
The last good thing written in C++ was the Pachelbel Canon. -- J. Olson
^ permalink raw reply
* Re: RFC: grafts generalised
From: Junio C Hamano @ 2008-07-03 0:03 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Stephen R. van den Berg, Linus Torvalds, git
In-Reply-To: <7vej6c7y8e.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> Yeah, thanks for a reminder.
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/37744/focus=37866
>>
>> is still on my "things to look at" list.
>
> This shows how the "object transfer ignores grafts" side of the earlier
> suggestion by Linus would look like to get people started. Totally
> untested.
>
> I threw in for_each_commit_graft() in the patch so that updates to the
> reachability walker can add otherwise hidden objects, but otherwise it is
> not used yet.
This updates the earlier patch to teach the object transfer side to ignore
grafts, which makes things consistent between dumb commit walkers and
native transport. It is not meant for application as I haven't thought
about[*1*] nor looked into how this may interact with the "shallow clone"
stuff (which is graft in disguise but implemented separately).
Footnote. *1* I also suspect Linus did not think about interactions with
"shallow" when he made the suggestion referenced above, as "shallow" was
still a relatively new curiosity back then.
I am not sure if the addition of --ignore-graft to revision.c should be
there when this becomes real. I added it primarily for debugging
purposes, as it is something the end users should never trigger in the
normal workflow.
--
builtin-pack-objects.c | 5 +++
builtin-send-pack.c | 3 +-
cache.h | 1 +
commit.c | 10 +++++++
commit.h | 2 +
environment.c | 1 +
revision.c | 4 +++
t/t6500-graft.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
upload-pack.c | 2 +
9 files changed, 97 insertions(+), 1 deletions(-)
create mode 100755 t/t6500-graft.sh
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 28207d9..53b0b33 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -30,6 +30,7 @@ git-pack-objects [{ -q | --progress | --all-progress }] \n\
[--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n\
[--stdout | base-name] [--include-tag] \n\
[--keep-unreachable | --unpack-unreachable] \n\
+ [--ignore-graft] \n\
[<ref-list | <object-list]";
struct object_entry {
@@ -2160,6 +2161,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
die("bad %s", arg);
continue;
}
+ if (!strcmp(arg, "--ignore-graft")) {
+ honor_graft = 0;
+ continue;
+ }
usage(pack_usage);
}
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index d76260c..d932352 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -27,6 +27,7 @@ static int pack_objects(int fd, struct ref *refs)
*/
const char *argv[] = {
"pack-objects",
+ "--ignore-graft",
"--all-progress",
"--revs",
"--stdout",
@@ -36,7 +37,7 @@ static int pack_objects(int fd, struct ref *refs)
struct child_process po;
if (args.use_thin_pack)
- argv[4] = "--thin";
+ argv[5] = "--thin";
memset(&po, 0, sizeof(po));
po.argv = argv;
po.in = -1;
diff --git a/cache.h b/cache.h
index 188428d..00858f9 100644
--- a/cache.h
+++ b/cache.h
@@ -435,6 +435,7 @@ extern size_t packed_git_limit;
extern size_t delta_base_cache_limit;
extern int auto_crlf;
extern int fsync_object_files;
+extern int honor_graft;
enum safe_crlf {
SAFE_CRLF_FALSE = 0,
diff --git a/commit.c b/commit.c
index e2d8624..62cf104 100644
--- a/commit.c
+++ b/commit.c
@@ -101,6 +101,13 @@ static int commit_graft_pos(const unsigned char *sha1)
return -lo - 1;
}
+void for_each_commit_graft(void (*fn)(struct commit_graft *))
+{
+ int i;
+ for (i = 0; i < commit_graft_nr; i++)
+ fn(commit_graft[i]);
+}
+
int register_commit_graft(struct commit_graft *graft, int ignore_dups)
{
int pos = commit_graft_pos(graft->sha1);
@@ -196,7 +203,10 @@ static void prepare_commit_graft(void)
struct commit_graft *lookup_commit_graft(const unsigned char *sha1)
{
int pos;
+
prepare_commit_graft();
+ if (!honor_graft)
+ return NULL;
pos = commit_graft_pos(sha1);
if (pos < 0)
return NULL;
diff --git a/commit.h b/commit.h
index 2d94d41..8f76dd9 100644
--- a/commit.h
+++ b/commit.h
@@ -138,4 +138,6 @@ static inline int single_parent(struct commit *commit)
return commit->parents && !commit->parents->next;
}
+void for_each_commit_graft(void (*fn)(struct commit_graft *));
+
#endif /* COMMIT_H */
diff --git a/environment.c b/environment.c
index 4a88a17..eb8f36d 100644
--- a/environment.c
+++ b/environment.c
@@ -41,6 +41,7 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
+int honor_graft = 1;
/* This is set by setup_git_dir_gently() and/or git_default_config() */
char *git_work_tree_cfg;
diff --git a/revision.c b/revision.c
index fc66755..25c96d0 100644
--- a/revision.c
+++ b/revision.c
@@ -991,6 +991,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
const char *arg = argv[i];
if (*arg == '-') {
int opts;
+ if (!strcmp(arg, "--ignore-graft")) {
+ honor_graft = 0;
+ continue;
+ }
if (!prefixcmp(arg, "--max-count=")) {
revs->max_count = atoi(arg + 12);
continue;
diff --git a/t/t6500-graft.sh b/t/t6500-graft.sh
new file mode 100755
index 0000000..122ed1c
--- /dev/null
+++ b/t/t6500-graft.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='graft and ancestry traversal'
+. test-lib.sh
+
+# Real history:
+#
+# .----D
+# /
+# A--B--C--E
+#
+# Grafted history:
+#
+# .----D
+# / \
+# A-----C--E
+#
+
+advance_one () {
+ echo "$1" >file &&
+ git add file &&
+ test_tick &&
+ git commit -m "$1" &&
+ git rev-parse --verify HEAD >"$1"
+}
+
+test_expect_success setup '
+ advance_one A &&
+ advance_one D &&
+
+ git reset --hard $(cat A) &&
+ advance_one B &&
+ advance_one C &&
+ advance_one E &&
+
+ (
+ echo $(cat E) $(cat C) $(cat D)
+ echo $(cat C) $(cat A)
+ ) >.git/info/grafts &&
+
+ git log --graph --pretty=oneline --abbrev-commit --parents &&
+ echo " - - - - - - - - - - " &&
+ git log --graph --pretty=oneline --abbrev-commit --parents --ignore-graft
+
+'
+
+test_expect_success 'clone should lose grafts' '
+ git clone --bare "file://$(pwd)/.git" cloned.git &&
+ (
+ GIT_DIR=cloned.git && export GIT_DIR &&
+ git log --graph --pretty=oneline --abbrev-commit --parents &&
+ git cat-file commit $(cat B) &&
+ test_must_fail git cat-file commit $(cat D) &&
+ git fsck --full
+ )
+'
+
+test_expect_success 'push should lose grafts' '
+ test_create_repo pushed.git &&
+ (
+ cd pushed.git &&
+ git fetch ../.git master:refs/heads/master &&
+ git log --graph --pretty=oneline --abbrev-commit --parents &&
+ git cat-file commit $(cat ../B) &&
+ test_must_fail git cat-file commit $(cat ../D) &&
+ git fsck --full
+ )
+'
+
+test_done
\ No newline at end of file
diff --git a/upload-pack.c b/upload-pack.c
index b46dd36..798caaa 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -158,6 +158,7 @@ static void create_pack_file(void)
die("git-upload-pack: unable to fork git-rev-list");
argv[arg++] = "pack-objects";
+ argv[arg++] = "--ignore-graft";
argv[arg++] = "--stdout";
if (!no_progress)
argv[arg++] = "--progress";
@@ -614,6 +615,7 @@ int main(int argc, char **argv)
int i;
int strict = 0;
+ honor_graft = 0;
for (i = 1; i < argc; i++) {
char *arg = argv[i];
^ permalink raw reply related
* finding deleted file names
From: Geoff Russell @ 2008-07-03 0:01 UTC (permalink / raw)
To: git
git diff --diff-filter=D --name-only HEAD@{'7 days ago'}
finds files deleted during the last 7 days, but if my repository is
only 6 days old I get a
fatal error.
fatal: bad object HEAD@{7 days ago}
Is there something that says "since repository creation", ie., go back as far
as possible, but no further? Is there a symbolic name for the initial commit?
Cheers,
Geoff Russell
^ permalink raw reply
* Re: RFC: grafts generalised
From: Stephan Beyer @ 2008-07-02 23:46 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Stephen R. van den Berg, git, Mike Hommey, Michael J Gruber
In-Reply-To: <37fcd2780807021342j75f351a5sa525b892caedf965@mail.gmail.com>
Hi,
On Thu, Jul 03, 2008 at 12:42:30AM +0400, Dmitry Potapov wrote:
> On Wed, Jul 2, 2008 at 11:31 PM, Stephan Beyer <s-beyer@gmx.net> wrote:
> > I wonder if grafts can be used in combination with sequencer in such a
> > way that you rewrite foo~20000..foo~19950 and then fake the parents of
> > foo~19949 to be the rewritten once.
>
> I don't think it is a good idea. During the normal work you should never
> use grafts.
I have written this in the context that Stephen only changes some commits
from a long time ago (foo~20000) and then I showed a way how to avoid that
sequencer rewrites the rest which takes so long.
This is not related to "normal work", but to Stephen's use case (if I
got it right).
What I've meant, was:
Instead of faking a lot of parents, changes and even merges using an
extended grafts file, he could rewrite some patches - which can be fast -
and then use _only one_ graft to change the parent to the changed and
rewritten commit.
This can be done iteratively and seems to be a good agreement in speed
and reliability.
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* [KORG] Master downtime
From: J.H. @ 2008-07-02 23:00 UTC (permalink / raw)
To: users, linux-kernel, support, git
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Afternoon everyone,
Just a quick heads up we are going to be taking master down for hardware
and software upgrades tomorrow, Thursday July 3rd 2008 at 15:00 UTC.
During this time all back-end services, including wiki's, ssh and
userweb to name a few things.
The hardware portion of the upgrade should be complete within an hour,
and the software upgrades will likely take another hour or two - though
even when master comes back up expect intermittent service for a few
hours after that while we deal with things. Likely ETA for all work
completed is Friday July 4th 2008 at 00:00 UTC. If there are
complications, I will kick another e-mail off with updates.
If you have any questions, comments or concerns please don't hesitate to
get ahold of me.
- - John 'Warthog9' Hawley
Chief Kernel.org Admin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
iD8DBQFIbAh3/E3kyWU9dicRAhOCAJ46E9rGDXQWwXCV932UEvo+cuqQwgCaAhQm
aWIIEVf7IFaTErGqHkx0Pag=
=woIy
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH 2/3] git-add--interactive: remove hunk coalescing
From: Junio C Hamano @ 2008-07-02 22:35 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Jeff King
In-Reply-To: <7vtzf77wjp.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Blindly concatenating the above two and feeding them to "git apply" *may*
> happen to work by accident, not by design. This very much feels like a
> hack of "This works most of the time for me, your mileage may vary" kind,
> which we would want to avoid when we can.
Well, I changed my mind. Let's run with this and see what happens.
The patch application is hunk-by-hunk in nature anyway, and if the user
munges the trailing context of the first half of an originally-single hunk
and the leading context of the latter half in an inconsistent way, we
would notice the problem anyway.
^ permalink raw reply
* Re: [PATCH 2/3] git-add--interactive: remove hunk coalescing
From: Junio C Hamano @ 2008-07-02 22:26 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Jeff King
In-Reply-To: <1215035984-26263-1-git-send-email-trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Current git-apply has no trouble at all applying chunks that have
> overlapping context, as produced by the splitting feature. So we can
> drop the manual coalescing.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> git-add--interactive.perl | 89 ---------------------------------------------
> 1 files changed, 0 insertions(+), 89 deletions(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index e1964a5..a4234d3 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -682,93 +682,6 @@ sub split_hunk {
> return @split;
> }
>
> -sub find_last_o_ctx {
> ...
> -}
> -
> -sub merge_hunk {
> ...
> -}
> -
> -sub coalesce_overlapping_hunks {
> ...
> -}
>
> sub help_patch_cmd {
> print colored $help_color, <<\EOF ;
> @@ -962,8 +875,6 @@ sub patch_update_file {
> }
> }
>
> - @hunk = coalesce_overlapping_hunks(@hunk);
> -
> my $n_lofs = 0;
> my @result = ();
> if ($mode->{USE}) {
> --
> 1.5.6.1.276.gde9a
I think [1/3] makes sense as we trust --recount anyway (and more
importantly if the user did not muck with the patch --recount would be a
no-op), but I am not sure about this one. I suspect this change reduces
the precision and safety of the patch application, especially when the
user does not edit hunks.
When you "[s]plit" a hunk like this:
@@ -n,7 +m,6 @@
con
text
-deleted preimage
+replaced postimage
more
line of context
-deleted another
context
into two, we prepare these two hunks internally:
@@ -n,5 +m,5 @@
con
text
-deleted preimage
+replaced postimage
more
line of context
@@ -l,4 +k,3 @@ -- l=n+5, k=m+5
more
line of context
-deleted another
context
So that applying only one piece without applying the other would still
have correct context to locate where to apply. However, if the user says
"I want to apply both after all", we would need to remove the overlap when
merge them back. If you don't, you would be feeding a nonsense patch to
"git apply" that goes back in context.
Blindly concatenating the above two and feeding them to "git apply" *may*
happen to work by accident, not by design. This very much feels like a
hack of "This works most of the time for me, your mileage may vary" kind,
which we would want to avoid when we can.
^ permalink raw reply
* [PATCH] git-send-email: Do not attempt to STARTTLS more than once
From: Thomas Rast @ 2008-07-02 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
With the previous TLS patch, send-email would attempt to STARTTLS at
the beginning of every mail, despite reusing the last connection. We
simply skip further encryption checks after successful TLS initiation.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
git-send-email.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index a047b01..3564419 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -772,6 +772,7 @@ X-Mailer: git-send-email $gitversion
if ($smtp->code == 220) {
$smtp = Net::SMTP::SSL->start_SSL($smtp)
or die "STARTTLS failed! ".$smtp->message;
+ $smtp_encryption = '';
} else {
die "Server does not support STARTTLS! ".$smtp->message;
}
--
1.5.6.1.278.g3d7f6
^ permalink raw reply related
* [PATCH 3/3] git-add--interactive: manual hunk editing mode
From: Thomas Rast @ 2008-07-02 22:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1215035984-26263-1-git-send-email-trast@student.ethz.ch>
Adds a new option 'e' to the 'add -p' command loop that lets you edit
the current hunk in your favourite editor.
If the resulting patch applies cleanly, the edited hunk will
immediately be marked for staging. If it does not apply cleanly, you
will be given an opportunity to edit again. If all lines of the hunk
are removed, then the edit is aborted and the hunk is left unchanged.
Applying the changed hunk(s) relies on Johannes Schindelin's new
--recount option for git-apply.
Note that the "real patch" test intentionally uses
(echo e; echo n; echo d) | git add -p
even though the 'n' and 'd' are superfluous at first sight. They
serve to get out of the interaction loop if git add -p wrongly
concludes the patch does not apply.
Many thanks to Jeff King <peff@peff.net> for lots of help and
suggestions.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-add.txt | 1 +
git-add--interactive.perl | 119 ++++++++++++++++++++++++++++++++++++++++++++
t/t3701-add-interactive.sh | 67 +++++++++++++++++++++++++
3 files changed, 187 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 011a743..46dd56c 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -236,6 +236,7 @@ patch::
k - leave this hunk undecided, see previous undecided hunk
K - leave this hunk undecided, see previous hunk
s - split the current hunk into smaller hunks
+ e - manually edit the current hunk
? - print help
+
After deciding the fate for all hunks, if there is any hunk
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index a4234d3..801d7c0 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -18,6 +18,18 @@ my ($fraginfo_color) =
$diff_use_color ? (
$repo->get_color('color.diff.frag', 'cyan'),
) : ();
+my ($diff_plain_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.plain', ''),
+ ) : ();
+my ($diff_old_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.old', 'red'),
+ ) : ();
+my ($diff_new_color) =
+ $diff_use_color ? (
+ $repo->get_color('color.diff.new', 'green'),
+ ) : ();
my $normal_color = $repo->get_color("", "reset");
@@ -683,6 +695,105 @@ sub split_hunk {
}
+sub color_diff {
+ return map {
+ colored((/^@/ ? $fraginfo_color :
+ /^\+/ ? $diff_new_color :
+ /^-/ ? $diff_old_color :
+ $diff_plain_color),
+ $_);
+ } @_;
+}
+
+sub edit_hunk_manually {
+ my ($oldtext) = @_;
+
+ my $hunkfile = $repo->repo_path . "/addp-hunk-edit.diff";
+ my $fh;
+ open $fh, '>', $hunkfile
+ or die "failed to open hunk edit file for writing: " . $!;
+ print $fh "# Manual hunk edit mode -- see bottom for a quick guide\n";
+ print $fh @$oldtext;
+ print $fh <<EOF;
+# ---
+# To remove '-' lines, make them ' ' lines (context).
+# To remove '+' lines, delete them.
+# Lines starting with # will be removed.
+#
+# If the patch applies cleanly, the edited hunk will immediately be
+# marked for staging. If it does not apply cleanly, you will be given
+# an opportunity to edit again. If all lines of the hunk are removed,
+# then the edit is aborted and the hunk is left unchanged.
+EOF
+ close $fh;
+
+ my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor")
+ || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
+ system('sh', '-c', $editor.' "$@"', $editor, $hunkfile);
+
+ open $fh, '<', $hunkfile
+ or die "failed to open hunk edit file for reading: " . $!;
+ my @newtext = grep { !/^#/ } <$fh>;
+ close $fh;
+ unlink $hunkfile;
+
+ # Abort if nothing remains
+ if (!grep { /\S/ } @newtext) {
+ return undef;
+ }
+
+ # Reinsert the first hunk header if the user accidentally deleted it
+ if ($newtext[0] !~ /^@/) {
+ unshift @newtext, $oldtext->[0];
+ }
+ return \@newtext;
+}
+
+sub diff_applies {
+ my $fh;
+ open $fh, '| git apply --recount --cached --check';
+ for my $h (@_) {
+ print $fh @{$h->{TEXT}};
+ }
+ return close $fh;
+}
+
+sub prompt_yesno {
+ my ($prompt) = @_;
+ while (1) {
+ print colored $prompt_color, $prompt;
+ my $line = <STDIN>;
+ return 0 if $line =~ /^n/i;
+ return 1 if $line =~ /^y/i;
+ }
+}
+
+sub edit_hunk_loop {
+ my ($head, $hunk, $ix) = @_;
+ my $text = $hunk->[$ix]->{TEXT};
+
+ while (1) {
+ $text = edit_hunk_manually($text);
+ if (!defined $text) {
+ return undef;
+ }
+ my $newhunk = { TEXT => $text, USE => 1 };
+ if (diff_applies($head,
+ @{$hunk}[0..$ix-1],
+ $newhunk,
+ @{$hunk}[$ix+1..$#{$hunk}])) {
+ $newhunk->{DISPLAY} = [color_diff(@{$text})];
+ return $newhunk;
+ }
+ else {
+ prompt_yesno(
+ 'Your edited hunk does not apply. Edit again '
+ . '(saying "no" discards!) [y/n]? '
+ ) or return undef;
+ }
+ }
+}
+
sub help_patch_cmd {
print colored $help_color, <<\EOF ;
y - stage this hunk
@@ -694,6 +805,7 @@ J - leave this hunk undecided, see next hunk
k - leave this hunk undecided, see previous undecided hunk
K - leave this hunk undecided, see previous hunk
s - split the current hunk into smaller hunks
+e - manually edit the current hunk
? - print help
EOF
}
@@ -798,6 +910,7 @@ sub patch_update_file {
if (hunk_splittable($hunk[$ix]{TEXT})) {
$other .= '/s';
}
+ $other .= '/e';
for (@{$hunk[$ix]{DISPLAY}}) {
print;
}
@@ -862,6 +975,12 @@ sub patch_update_file {
$num = scalar @hunk;
next;
}
+ elsif ($line =~ /^e/) {
+ my $newhunk = edit_hunk_loop($head, \@hunk, $ix);
+ if (defined $newhunk) {
+ splice @hunk, $ix, 1, $newhunk;
+ }
+ }
else {
help_patch_cmd($other);
next;
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index fae64ea..e95663d 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -66,6 +66,73 @@ test_expect_success 'revert works (commit)' '
grep "unchanged *+3/-0 file" output
'
+cat >expected <<EOF
+EOF
+cat >fake_editor.sh <<EOF
+EOF
+chmod a+x fake_editor.sh
+test_set_editor "$(pwd)/fake_editor.sh"
+test_expect_success 'dummy edit works' '
+ (echo e; echo a) | git add -p &&
+ git diff > diff &&
+ test_cmp expected diff
+'
+
+cat >patch <<EOF
+@@ -1,1 +1,4 @@
+ this
++patch
+-doesn't
+ apply
+EOF
+echo "#!$SHELL_PATH" >fake_editor.sh
+cat >>fake_editor.sh <<\EOF
+mv -f "$1" oldpatch &&
+mv -f patch "$1"
+EOF
+chmod a+x fake_editor.sh
+test_set_editor "$(pwd)/fake_editor.sh"
+test_expect_success 'bad edit rejected' '
+ git reset &&
+ (echo e; echo n; echo d) | git add -p >output &&
+ grep "hunk does not apply" output
+'
+
+cat >patch <<EOF
+this patch
+is garbage
+EOF
+test_expect_success 'garbage edit rejected' '
+ git reset &&
+ (echo e; echo n; echo d) | git add -p >output &&
+ grep "hunk does not apply" output
+'
+
+cat >patch <<EOF
+@@ -1,0 +1,0 @@
+ baseline
++content
++newcontent
++lines
+EOF
+cat >expected <<EOF
+diff --git a/file b/file
+index b5dd6c9..f910ae9 100644
+--- a/file
++++ b/file
+@@ -1,4 +1,4 @@
+ baseline
+ content
+-newcontent
++more
+ lines
+EOF
+test_expect_success 'real edit works' '
+ (echo e; echo n; echo d) | git add -p &&
+ git diff >output &&
+ test_cmp expected output
+'
+
if test "$(git config --bool core.filemode)" = false
then
say 'skipping filemode tests (filesystem does not properly support modes)'
--
1.5.6.1.276.gde9a
^ permalink raw reply related
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