[PATCH v2 21/23] rebase -i: teach do_pick the option --reset-author
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement many of the to-do list commands.
Eventually, the complete `do_pick` interface will be exposed to the
user in some form or another and those commands will become simple
aliases for the `do_pick` options now used to implement them.
Add the git-commit option `--reset-author` to the options pool of
`do_pick`. It rewrites the author date and name of the picked commit
to match the committer date and name.
If `--reset-author` is passed to `do_pick`, set the `rewrite` flag
and relay the option to the git-commit command line which creates the
final commit. If `--amend` is not passed as well, the fresh
authorship effect is achieved by the mere fact that we are creating a
new commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index da435cb..d6c99ea 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,10 +464,18 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--amend] [--file file] [--edit] commit
+# do_pick [--reset-author] [--amend] [--file file] [--edit] commit
#
# Wrapper around git-cherry-pick.
#
+# --reset-author
+# Pretend the changes were made for the first time. Declare that the
+# authorship of the resulting commit now belongs to the committer.
+# This also renews the author timestamp. This creates a fresh
+# commit.
+#
+# _This is not a git-cherry-pick option._
+#
# --amend
# After picking commit, replace the current head commit with a new
# commit that also introduces the changes of commit.
@@ -502,6 +510,10 @@ do_pick () {
while test $# -gt 0
do
case $1 in
+ --reset-author)
+ rewrite=y
+ rewrite_author=y
+ ;;
--amend)
if test $(git rev-parse HEAD) = $squash_onto || !
git rev-parse -q --verify HEAD /dev/null
then
@@ -564,6 +576,14 @@ do_pick () {
pick_one ${rewrite:+-n} $1 || return 1
fi
+ if test -n $rewrite_author test -z $rewrite_amend
+ then
+ # keep rewrite flag to create a new commit, rewrite
+ # without --reset-author though because it can only be
+ # used with -C, -c or --amend
+ rewrite_author=
+ fi
+
if test -n $rewrite
then
output git commit --allow-empty --no-post-rewrite -n --no-edit \
@@ -571,6 +591,7 @@ do_pick () {
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit --commit-msg} \
${rewrite_message:+--file $rewrite_message} \
+ ${rewrite_author:+--reset-author} \
${gpg_sign_opt:+$gpg_sign_opt} || return 3
fi
}
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/23] rebase -i: log the replay of root commits
The command line used to recreate root commits specifies the option
`-q` which suppresses the commit summary message. However,
git-rebase--interactive tends to tell the user about the commits it
creates in the final history, if she wishes (cf. command line option
`--verbose`). The code parts handling non-root commits and squash
commits all output commit summary messages. Do not make the replay of
root commits an exception. Remove the option to make the report of
the rebased history complete. Do not forget to wrap the git-commit
command line in `output` so that the summary is shown if git-rebase
is called with the `--verbose` option but suppressed otherwise.
It is OK that the commit summary is still suppressed when git-commit
is used to initialize the authorship of the sentinel commit because
this additional commit is an implementation detail hidden from the
final history. The removed `-q` option was probably introduced as a
copy-and-paste error stemming from that part of the root commit
handling code.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8e1730c..91ef0f7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -510,8 +510,8 @@ do_pick () {
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1
pick_one -n $1
- git commit --allow-empty --allow-empty-message \
- --amend --no-post-rewrite -n -q -C $1 \
+ output git commit --allow-empty --allow-empty-message \
+ --amend --no-post-rewrite -n -C $1 \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/23] rebase -i: prepare for squash in terms of do_pick --amend
Rewrite `squash` and `fixup` handling in `do_next` using the sequence
pick_one
commit
in order to test the correctness of a single `do_squash` or
parameterised `do_pick` and make the subsequent patch reimplementing
`squash` in terms of such a single function more readable.
Do not call `rm -f $GIT_DIR/MERGE_MSG` since it has no effect on
the state after git-rebase--interactive terminates. The option `-F`
makes git-commit ignore `MERGE_MSG` for the log message. If
git-commit succeeds, `MERGE_MSG` is removed, and if it fails,
`MERGE_MSG` is overwritten by the error sequence `die_failed_squash`.
In summary, removing `MERGE_MSG` neither influences the squash commit
message nor the file state after git-commit returns.
Moreover, `pick_one` ignores `$GIT_DIR/SQUASH_MSG` and does not touch
`$squash_msg` so that it is correct to execute `pick_one` immediately
before git-commit.
Might be squashed into the subsequent commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0fbf773..601a2ff 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -623,15 +623,15 @@ do_next () {
author_script_content=$(get_author_ident_from_commit HEAD)
echo $author_script_content $author_script
eval $author_script_content
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 $rest
- fi
case $(peek_next_command) in
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 Could not apply
$sha1... $rest
+ fi
do_with_author output git commit --allow-empty-message
--allow-empty \
--amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
@@ -641,13 +641,22 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 Could not
apply $sha1... $rest
+ fi
do_with_author output git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
- rm -f $GIT_DIR/MERGE_MSG
+ if ! pick_one -n $sha1
+ then
+ git rev-parse --verify HEAD $amend
+ die_failed_squash $sha1 Could not
apply $sha1... $rest
+ fi
do_with_author output git commit --allow-empty
--amend --no-pre-commit -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 22/23] rebase -i: teach do_pick the option --signoff
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is currently used to implement most of the to-do list commands
and offers additional options that will eventually find their way
onto to-do lists.
To extend the repertoire of available options, add the git-commit and
git-cherry-pick option `--signoff` to the `do_pick` interface. It
appends a Signed-off-by: line using the committer identity to the log
message of the picked commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d6c99ea..a22459f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,10 +464,15 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--reset-author] [--amend] [--file file] [--edit] commit
+# do_pick [--signoff] [--reset-author] [--amend] [--file file]
+# [--edit] commit
#
# Wrapper around git-cherry-pick.
#
+# -s, --signoff
+# Insert a Signed-off-by: line using the committer identity at the
+# end of the commit log message. This creates a fresh commit.
+#
# --reset-author
# Pretend the changes were made for the first time. Declare that the
# authorship of the resulting commit now belongs to the committer.
@@ -510,6 +515,10 @@ do_pick () {
while test $# -gt 0
do
case $1 in
+ -s|--signoff)
+ rewrite=y
+ rewrite_signoff=y
+ ;;
--reset-author)
rewrite=y
rewrite_author=y
@@ -588,6 +597,7 @@ do_pick () {
then
output git commit --allow-empty --no-post-rewrite -n --no-edit \
${allow_empty_message:+--allow-empty-message} \
+ ${rewrite_signoff:+--signoff} \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit --commit-msg} \
${rewrite_message:+--file $rewrite_message} \
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/23] rebase -i: implement reword in terms of do_pick
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If one thinks of `pick`
entries as scheduled `cherry-pick` command lines, then `reword`
becomes an alias for the command line `cherry-pick --edit`. The
porcelain `rebase--interactive` defines a function `do_pick` for
processing the `pick` entries on to-do lists. Reimplement `reword` in
terms of `do_pick --edit`.
If the user picks a commit using the to-do list line
reword fa1afe1 Some change
execute the command `do_pick --edit fa1afe1 Some change` which
carries out exactly the same steps as the case arm for `reword` in
`do_next` so far.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index aed2f93..8e1730c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -546,11 +546,7 @@ do_next () {
comment_for_reflog reword
mark_action_done
- do_pick $sha1 $rest
- output git commit --allow-empty --amend --no-post-rewrite
--no-pre-commit ${gpg_sign_opt:+$gpg_sign_opt} || {
- warn Could not amend commit after successfully picking
$sha1... $rest
- exit_with_patch $sha1 1
- }
+ do_pick --edit $sha1 $rest
record_in_rewritten $sha1
;;
edit|e)
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/23] rebase -i: root commits are replayed with an unnecessary option
The command line used to recreate root commits uses `-C` to suppress
the log message editor. This is unnecessarily confusing, though,
because that suppression is a secondary effect of the option. The
main purpose of `-C` is to pull the metadata from another commit, but
here we know that this is a noop, since we are amending a commit just
created from the same data.
At the time `-C` was introduced, git-commit did not yet have a
documented `--no-edit`, and this was a reasonable way to get the
desired behavior. Switch it to use `--no-edit` to make the intended
effect more obvious.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 91ef0f7..71571c8 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -511,7 +511,7 @@ do_pick () {
--no-post-rewrite -n -q -C $1
pick_one -n $1
output git commit --allow-empty --allow-empty-message \
- --amend --no-post-rewrite -n -C $1 \
+ --amend --no-post-rewrite -n --no-edit \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/23] rebase -i: squash skips commit-msg hook
Using the to-do list command `squash` the user can specify two or
more commits and git-rebase creates one commit that introduces all
their changes combined. The authorship for the created commit is
taken from the first commit specified and the user can edit the log
message. There is a variant of `squash` available named `fixup` which
also takes the first log message without asking for user input.
While it is reasonable to not verify replayed changes twice or
rejecting some other author's changes in her name, it is insufficient
to not verify the user input used as log message in the case of
`squash`. Specify the git-commit option `--commit-msg` when
committing the squash result to execute the commit-msg hook and
verify the new log message. For the same reasons the pre-commit hook
is disabled in all replay modes, the commit-msg hook gets disabled in
`fixup` mode.
Add tests. In addition to the existing test checking that the
pre-commit hook is disabled when simply picking a commit, provide a
test checking that the commit-msg hook is disabled as well.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 2 +-
t/t3404-rebase-interactive.sh | 78 +++
2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3ee13c2..97386aa 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -562,7 +562,7 @@ do_next () {
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author output git commit --allow-empty
--amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author output git commit --allow-empty
--amend --no-pre-commit -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
fi
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c6578c3..bc2dda1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -655,6 +655,84 @@ test_expect_success 'rebase a commit violating pre-commit'
'
'
+test_expect_success 'setup failing pre-commit' '
+ HOOKDIR=$(git rev-parse --git-dir)/hooks
+ mkdir -p $HOOKDIR
+ PRE_COMMIT=$HOOKDIR/pre-commit
+ cat $PRE_COMMIT -EOF
+ #!/bin/sh
+ echo running failing pre-commit...
+ exit 1
+ EOF
+ chmod +x $PRE_COMMIT
+ git checkout -b violating-pre-commit master
+ test_must_fail test_commit pre-commit-violated-1
+ test_commit --no-verify pre-commit-violated-1
+ test_must_fail test_commit pre-commit-violated-2
+ test_commit --no-verify pre-commit-violated-2
+'
+
+test_expect_success 'squash a commit violating pre-commit' '
+ git checkout -b squash-violating-pre-commit violating-pre-commit
+ test_when_finished reset_rebase
+ set_fake_editor
+ env FAKE_LINES=1 squash 2 git rebase -i master
+'
+
+test_expect_success 'fixup a commit violating pre-commit' '
+ git checkout -b fixup-violating-pre-commit violating-pre-commit
+ test_when_finished reset_rebase
+ set_fake_editor
+ env FAKE_LINES=1 fixup 2 git rebase -i master
+'
+
+test_expect_success 'clean up failing pre-commit' '
+ rm $PRE_COMMIT
+'
+
+test_expect_success 'setup failing commit-msg' '
+ HOOKDIR=$(git rev-parse --git-dir)/hooks
+ mkdir -p $HOOKDIR
+ COMMIT_MSG=$HOOKDIR/commit-msg
+ cat $COMMIT_MSG -EOF
+ #!/bin/sh
+ echo running failing commit-msg...
+ exit 1
+ EOF
+ chmod +x $COMMIT_MSG
+ git checkout -b violating-commit-msg master
+ test_must_fail test_commit commit-msg-violated-1
+ test_commit --no-verify commit-msg-violated-1
+ test_must_fail test_commit commit-msg-violated-2
+ test_commit --no-verify commit-msg-violated-2
+ test_must_fail test_commit commit-msg-violated-3
+ test_commit --no-verify commit-msg-violated-3
+'
+
+test_expect_success 'rebase a commit violating commit-msg' '
+ git checkout -b rebase-violating-commit-msg violating-commit-msg
+ set_fake_editor
+ FAKE_LINES=1 git rebase -i master
+'
+
+test_expect_success 'squash a commit violating commit-msg' '
+ git checkout -b squash-violating-commit-msg violating-commit-msg
+ set_fake_editor
+ test_must_fail env FAKE_LINES=1 squash 2 fixup 3 git rebase -i master
+ git commit --no-verify --amend
+ git rebase --continue
+'
+
+test_expect_success 'fixup a commit violating commit-msg' '
+ git checkout -b fixup-violating-commit-msg violating-commit-msg
+ set_fake_editor
+ env FAKE_LINES=1 fixup 2 git rebase -i master
+'
+
[PATCH v2 14/23] rebase -i: do not die in do_pick
Since `do_pick` might be executed in a sub-shell (a modified author
environment for instance), calling `die` in `do_pick` has no effect
but exiting the sub-shell with a failure exit status. The
git-rebase--interactive script is not terminated. Moreover, if
`do_pick` is called while a squash or fixup is in effect,
`die_with_patch` will discard `$squash_msg` as commit message.
Lastly, after a `die` in `do_pick` `do_next` has no chance to
reschedule tasks that failed before changes could be applied.
Indicate an error in `do_pick` using return statements and properly
kill the script at the call sites. Although possible in principle,
the issued error messages are no more indicating whether `do_pick`
failed while applying or while committing the changes. This reduces
code complexity at the call site and does not matter from a user's
point of view because a glance at the index reveals whether there are
conflicts or not and in-depth troubleshooting is still possible using
the `--verbose` option.
Remove the commit message title argument from `do_pick`'s interface,
which has become unused.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b8c734e..d812bad 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,7 +464,7 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--edit] commit title
+# do_pick [--edit] commit
#
# Wrapper around git-cherry-pick.
#
@@ -476,9 +476,11 @@ record_in_rewritten() {
# commit
# The commit to cherry-pick.
#
-# title
-# The commit message title of commit. Used for information
-# purposes only.
+# The return value is 1 if applying the changes resulted in a conflict
+# and 2 if the specified arguments were incorrect. If the changes could
+# be applied successfully but creating the commit failed, a value
+# greater than 2 is returned. No commit is created in either case and
+# the index is left with the (conflicting) changes in place.
do_pick () {
allow_empty_message=y
rewrite=
@@ -493,7 +495,8 @@ do_pick () {
allow_empty_message=
;;
-*)
- die do_pick: unrecognized option -- $1
+ warn do_pick: unrecognized option -- $1
+ return 2
;;
*)
break
@@ -501,7 +504,11 @@ do_pick () {
esac
shift
done
- test $# -ne 2 die do_pick: wrong number of arguments
+ if test $# -ne 1
+ then
+ warn do_pick: wrong number of arguments
+ return 2
+ fi
if test $(git rev-parse HEAD) = $squash_onto
then
@@ -519,11 +526,9 @@ do_pick () {
# rebase --continue.
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1
- pick_one -n $1 ||
- die_with_patch $1 Could not apply $1... $2
+ pick_one -n $1 || return 1
else
- pick_one ${rewrite:+-n} $1 ||
- die_with_patch $1 Could not apply $1... $2
+ pick_one ${rewrite:+-n} $1 || return 1
fi
if test -n $rewrite
@@ -532,8 +537,7 @@ do_pick () {
${allow_empty_message:+--allow-empty-message} \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit --commit-msg} \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_with_patch $1 Could not rewrite commit after
successfully picking $1... $2
+ ${gpg_sign_opt:+$gpg_sign_opt} || return 3
fi
}
@@ -548,21 +552,21 @@ do_next () {
comment_for_reflog pick
mark_action_done
- do_pick $sha1 $rest
+ do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
record_in_rewritten $sha1
;;
reword|r)
comment_for_reflog reword
mark_action_done
- do_pick --edit $sha1 $rest
+ do_pick --edit $sha1 || die_with_patch $sha1 Could not apply
$sha1... $rest
record_in_rewritten $sha1
;;
edit|e)
comment_for_reflog edit
mark_action_done
- do_pick $sha1 $rest
+ do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
warn Stopped at $sha1... $rest
exit_with_patch $sha1 0
;;
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe
[PATCH v2 19/23] rebase -i: explicitly distinguish replay commands and exec tasks
There are two kinds of to-do list commands available. One kind
replays a commit (`pick`, `reword`, `edit`, `squash` and `fixup` that
is) and the other executes a shell command (`exec`). We will call the
first kind replay commands.
The two kinds of tasks are scheduled using different line formats.
Replay commands expect a commit hash argument following the command
name and exec concatenates all arguments to assemble a command line.
Adhere to the distinction of formats by not trying to parse the
`sha1` field unless we are dealing with a replay command. Move the
replay command handling code to a new function `do_replay` which
assumes the first argument to be a commit hash and make no more such
assumptions in `do_next`.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 42 --
1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 29eca7e..6ecd4d7 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -575,13 +575,12 @@ do_pick () {
fi
}
-do_next () {
- rm -f $msg $author_script $amend || exit
- read -r command sha1 rest $todo
+do_replay () {
+ command=$1
+ sha1=$2
+ rest=$3
+
case $command in
- $comment_char*|''|noop)
- mark_action_done
- ;;
pick|p)
comment_for_reflog pick
@@ -660,6 +659,28 @@ do_next () {
esac
record_in_rewritten $sha1
;;
+ *)
+ read -r command $todo
+ warn Unknown command: $command
+ fixtodo=Please fix this using 'git rebase --edit-todo'.
+ if git rev-parse --verify -q $sha1 /dev/null
+ then
+ die_with_patch $sha1 $fixtodo
+ else
+ die $fixtodo
+ fi
+ ;;
+ esac
+}
+
+do_next () {
+ rm -f $msg $author_script $amend || exit
+ read -r command sha1 rest $todo
+
+ case $command in
+ $comment_char*|''|noop)
+ mark_action_done
+ ;;
x|exec)
read -r command rest $todo
mark_action_done
@@ -699,14 +720,7 @@ do_next () {
fi
;;
*)
- warn Unknown command: $command $sha1 $rest
- fixtodo=Please fix this using 'git rebase --edit-todo'.
- if git rev-parse --verify -q $sha1 /dev/null
- then
- die_with_patch $sha1 $fixtodo
- else
- die $fixtodo
- fi
+ do_replay $command $sha1 $rest
;;
esac
test -s $todo return
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/23] rebase -i: commit only once when rewriting picks
The options passed to `do_pick` determine whether the picked commit
will be rewritten or not. If the commit gets rewritten, because the
user requested to edit the commit message for instance, let
`pick_one` merely apply the changes introduced by the commit and do
not commit the resulting tree yet. If the commit is replayed as is,
leave it to `pick_one` to recreate the commit (possibly by
fast-forwarding the head). This makes it easier to combine git-commit
options like `--edit` and `--amend` in `do_pick` because
git-cherry-pick does not support `--amend`.
In the case of `--edit`, do not `exit_with_patch` but assign
`rewrite` to pick the changes with `-n`. If the pick conflicts, no
commit is created which we would have to amend when continuing the
rebase. To complete the pick after the conflicts are resolved the
user just resumes with `git rebase --continue`.
git-commit lets the user edit the commit log message by default. We
do not want that for the rewriting git-commit command line because
the default behaviour of git-rebase is exactly the opposite. Pass
`--no-edit` when rewriting a picked commit. An explicit `--edit`
passed to `do_pick` (for instance, when reword is executed) enables
the editor launch again. Similarly, pass `--allow-empty-message`
unless the log message is edited.
If `rebase--interactive` is used to rebase a complete branch onto
some head, `rebase` creates a sentinel commit that requires special
treatment by `do_pick`. Do not finalize the pick here either because
its commit message can be altered as for any other pick. Since the
orphaned root commit gets a temporary parent, it is always rewritten.
Safely use the rewrite infrastructure of `do_pick` to create the
final commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 71571c8..b8c734e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -63,7 +63,8 @@ msgnum=$state_dir/msgnum
author_script=$state_dir/author-script
# When an edit rebase command is being processed, the SHA1 of the
-# commit to be edited is recorded in this file. When git rebase
+# commit to be edited is recorded in this file. The same happens when
+# rewriting a commit fails, for instance reword. When git rebase
# --continue is executed, if there are any staged changes then they
# will be amended to the HEAD commit, but only provided the HEAD
# commit is still the commit to be edited. When any other rebase
@@ -479,12 +480,17 @@ record_in_rewritten() {
# The commit message title of commit. Used for information
# purposes only.
do_pick () {
- edit=
+ allow_empty_message=y
+ rewrite=
+ rewrite_amend=
+ rewrite_edit=
while test $# -gt 0
do
case $1 in
-e|--edit)
- edit=y
+ rewrite=y
+ rewrite_edit=y
+ allow_empty_message=
;;
-*)
die do_pick: unrecognized option -- $1
@@ -499,6 +505,10 @@ do_pick () {
if test $(git rev-parse HEAD) = $squash_onto
then
+ rewrite=y
+ rewrite_amend=y
+ git rev-parse --verify HEAD $amend
+
# Set the correct commit message and author info on the
# sentinel root before cherry-picking the original changes
# without committing (-n). Finally, update the sentinel again
@@ -509,22 +519,21 @@ do_pick () {
# rebase --continue.
git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1
- pick_one -n $1
- output git commit --allow-empty --allow-empty-message \
- --amend --no-post-rewrite -n --no-edit \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
+ pick_one -n $1 ||
die_with_patch $1 Could not apply $1... $2
else
- pick_one $1 ||
+ pick_one ${rewrite:+-n} $1 ||
die_with_patch $1 Could not apply $1... $2
fi
- if test -n $edit
+ if test -n $rewrite
then
- output git commit --allow-empty --amend --no-post-rewrite
--no-pre-commit ${gpg_sign_opt:+$gpg_sign_opt} || {
- warn Could not amend commit after successfully picking
$1... $2
- exit_with_patch $1 1
- }
+ output git commit --allow-empty --no-post-rewrite -n --no-edit \
+ ${allow_empty_message:+--allow-empty-message} \
+ ${rewrite_amend:+--amend} \
+
[PATCH v2 18/23] rebase -i: implement squash in terms of do_pick
The to-do list command `squash` and its close relative `fixup` replay
the changes of a commit like `pick` but do not recreate the commit.
Instead they replace the previous commit with a new commit that also
introduces the changes of the squashed commit. This is roughly like
cherry-picking without committing and using git-commit to amend the
previous commit.
The to-do list
pick a Some changes
squash b Some more changes
gets translated into the sequence of git commands
git cherry-pick a
git cherry-pick -n b
git commit --amend
and if git-cherry-pick supported `--amend` this would look even more
like the to-do list it is based on
git cherry-pick a
git cherry-pick --amend b.
Since `do_pick` takes care of `pick` entries and the above suggests
`squash` as an alias for `pick --amend`, reimplement `squash` in
terms of `do_pick --amend`. Introduce `$squash_msg` as the commit
message via the `--file` option. When the last commit of a squash
series is processed, the user is asked to review the log message.
Pass `--edit` as additional option in this case. The only difference
in the options passed to git-commit and `do_pick` is the omitted
`--no-verify`. However, `do_pick` does not execute the verification
hooks anyway because it solely replays commits and assumes that they
have been verified before.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 32 ++--
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 601a2ff..29eca7e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -627,39 +627,19 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 Could not apply
$sha1... $rest
- fi
- do_with_author output git commit --allow-empty-message
--allow-empty \
- --amend --no-verify -F $squash_msg \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_failed_squash $sha1 $rest
+ do_with_author do_pick --amend -F $squash_msg $sha1 \
+ || die_failed_squash $sha1 $rest
;;
*)
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 Could not
apply $sha1... $rest
- fi
- do_with_author output git commit
--allow-empty-message --allow-empty \
- --amend --no-verify -F $fixup_msg \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_failed_squash $sha1 $rest
+ do_with_author do_pick --amend -F $fixup_msg
$sha1 \
+ || die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
- if ! pick_one -n $sha1
- then
- git rev-parse --verify HEAD $amend
- die_failed_squash $sha1 Could not
apply $sha1... $rest
- fi
- do_with_author output git commit --allow-empty
--amend --no-pre-commit -F $GIT_DIR/SQUASH_MSG -e \
- ${gpg_sign_opt:+$gpg_sign_opt} ||
- die_failed_squash $sha1 $rest
+ do_with_author do_pick --amend -F
$GIT_DIR/SQUASH_MSG -e $sha1 \
+ || die_failed_squash $sha1 $rest
fi
rm -f $squash_msg $fixup_msg
if test -z $keep_empty is_empty_commit HEAD
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 23/23] rebase -i: enable options --signoff, --reset-author for pick, reword
pick and reword are atomic to-do list commands in the sense that they
open a new task which is closed after the respective command is
completed. squash and fixup are not atomic. They create a new task
which is not completed until the last squash or fixup is processed.
Lift the general unknown option blockade for the pick and reword
commands. If `do_cmd` comes across one of the options `--signoff` and
`--reset-author` while parsing a to-do entry and the scheduled
command is either `pick` or `reword`, relay the option to `do_pick`.
The `do_pick` options `--gpg-sign` and `--file` are not yet supported
because `do_cmd` cannot handle option arguments and options with
spaces at the moment. It is true that edit is one of the atomic
commands but it displays hash information when the rebase is stopped
and some options rewrite the picked commit which alters that
information. squash and fixup still do not accept user options as the
interplay of `--reset-author` and the author script are yet to be
determined.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a22459f..4c05734 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -614,6 +614,16 @@ do_replay () {
while test $# -gt 0
do
case $1 in
+ --signoff|--reset-author)
+ case $command in
+ pick|reword)
+ ;;
+ *)
+ warn Unsupported option: $1
+ command=unknown
+ ;;
+ esac
+ ;;
-*)
warn Unknown option: $1
command=unknown
@@ -634,21 +644,24 @@ do_replay () {
comment_for_reflog pick
mark_action_done
- do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
+ eval do_pick $opts $sha1 \
+ || die_with_patch $sha1 Could not apply $sha1... $rest
record_in_rewritten $sha1
;;
reword|r)
comment_for_reflog reword
mark_action_done
- do_pick --edit $sha1 || die_with_patch $sha1 Could not apply
$sha1... $rest
+ eval do_pick --edit $opts $sha1 \
+ || die_with_patch $sha1 Could not apply $sha1... $rest
record_in_rewritten $sha1
;;
edit|e)
comment_for_reflog edit
mark_action_done
- do_pick $sha1 || die_with_patch $sha1 Could not apply $sha1...
$rest
+ eval do_pick $opts $sha1 \
+ || die_with_patch $sha1 Could not apply $sha1... $rest
warn Stopped at $sha1... $rest
exit_with_patch $sha1 0
;;
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 09/19] rebase -i: commit only once when rewriting picks
Hi Jeff, Jeff King writes: On Tue, Jul 29, 2014 at 01:18:09AM +0200, Fabian Ruch wrote: The options passed to `do_pick` determine whether the picked commit will be rewritten or not. If the commit gets rewritten, because the user requested to edit the commit message for instance, let `pick_one` merely apply the changes introduced by the commit and do not commit the resulting tree yet. If the commit is replayed as is, leave it to `pick_one` to recreate the commit (possibly by fast-forwarding the head). This makes it easier to combine git-commit options like `--edit` and `--amend` in `do_pick` because git-cherry-pick does not support `--amend`. In the case of `--edit`, do not `exit_with_patch` but assign `rewrite` to pick the changes with `-n`. If the pick conflicts, no commit is created which we would have to amend when continuing the rebase. To complete the pick after the conflicts are resolved the user just resumes with `git rebase --continue`. Hmm. So does this mean the user will actually see a different state during such a conflict? E.g., if I have instructions like: pick A squash B squash C and there is a conflict picking C, then what state do I see? Right now I see a commit with the A+B squash prepared. But your description sounds to me like we would avoid the squash for B, and the user would see a different state. The squash state will not be different. squash sequences are still taken care of one line after the other: committing A, committing A+B, committing A+B+C. If there is a conflict picking C, HEAD will point to A+B and the index will record the conflicting changes. However, I couldn't trigger that behavior in a few experiments. Am I misunderstanding, or is there some case where the user-visible state will be different? The user-visible state will be different for rewords. For instance, let's consider pick A reword B. The verbose log used to show two commits for B (with ff disabled), one after picking and one after editing. Now the log will show a single commit in connection with 'reword B' which might be less confusing. Thanks for raising your eyebrows. I noticed now that the last paragraph is plainly wrong. The described amend situation did not arise if the pick conflicted but if the edited commit did not verify. There will be no after the conflicts are resolved but the user can either commit manually and circumvent log message verification if she knows what she's doing, or provide a corrected log message in the editor launched by 'git rebase --continue'. The _incomplete_ 'git commit --amend' tip which used to be displayed after a failed verification hook could become unnecessary and this would possibly spare us including correct GPG sign options for instance. However, this patch is mostly motivated by the unification of how commits are rewritten. Before, rewords and squashes went about this differently, now both fail with an uncommitted index if there are conflicts or the log message is ill-formatted. The log message must be corrected and the following bug, which was noticed after PATCH v2, must be fixed. cat .git/hooks/commit-msg -EOF #!/bin/sh exit 1 EOF chmod +x .git/hooks/commit-msg test_must_fail env FAKE_LINES=reword 1 git rebase -i test_must_fail git rebase --continue # the last command succeeds because --continue does not verify Fabian -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#757297: 'git status' output is confusing after 'git add -N'
On Thu, Aug 7, 2014 at 7:34 AM, Jonathan Nieder jrnie...@gmail.com wrote: Package: git Version: 1:2.0.0-1 Tags: upstream $ git init foo Initialized empty Git repository in /tmp/t/foo/.git/ $ cd foo $ echo hi README $ git add -N README $ git status On branch master Initial commit Changes to be committed: (use git rm --cached file... to unstage) new file: README Changes not staged for commit: (use git add file... to update what will be committed) (use git checkout -- file... to discard changes in working directory) modified: README If I then run git commit, it does not actually commit the addition of the README file. We used to reject such a commit operation before 3f6d56d (commit: ignore intent-to-add entries instead of refusing - 2012-02-07) so it was harder to misunderstand this case. It would be clearer to have a separate section,like so: Tracked files not to be committed: (use git rm --cached file... to stop tracking) new file: README Or make the Changes not staged for commit part say new file: README (modified is implied) -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
glob escaping doesn't work with git mv
I tried using escaped globs with git mv, but globs don't seem to be expanded with git mv. So, for example, I've got files file1 file2, and have been editing them, so I've got file1~ and file2~ also, and *~ is in .gitignore. If I do: $ mkdir newdir $ git mv file* newdir I get a 'fatal: not in repository' error because I'm trying to move untracked files. Since we can do git add file\*, I tried: $ git mv file\* newdir I get a 'bad source, source=file* destination=file*' error. It seems to be looking for a file named 'file*' rather than performing glob expansion. If I do: $ rm *~ $ git mv file* newdir Everything works as expected. It would be convenient if git mv expanded escaped globs. Thanks! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: glob escaping doesn't work with git mv
On Thu, Aug 7, 2014 at 8:52 AM, Alan Grover alan.gro...@gmail.com wrote: Since we can do git add file\*, I tried: $ git mv file\* newdir I get a 'bad source, source=file* destination=file*' error. It seems to be looking for a file named 'file*' rather than performing glob expansion. This is a known problem (to me at least). Source path processing in git-mv is a bit complicated and it scared me away from converting to using pathspec (which supports globbing and stuff). But perhaps if I just expand pathspec and feed the final path list to git-mv (pretty much like how shells expand glob) then it probably works.. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Rebase safely (Re: cherry picking and merge)
On Wed, Aug 06, 2014 at 05:38:43PM -0700, Mike Stump wrote:
Oh, wait, maybe I have misunderstood the prohibition. I have:
upstream —— fsf
|
\
|
v
Me — master — coworker.
This looks a lot like what I meant about project repos.
Me is a git clone of master, coworker is a git clone of master.
Master is a bare repo on a shared server where we put all of our work.
upstream is a bare git repo of the fsf git tree for gcc. fsf is a box
Yes, exactly. We did used exactly this at Sun, with a rebase-only
workflow. You won't believe it till you see it [below].
owned by other that hosts the gcc git repository. I do a git pull fsf
in upstream from time to time, and a corresponding git merge fsf in Me
from time to time. When I like my work I do a git push (to master
exclusively). To go to upstream, we submit patches by hand, git is
not really involved. I never pull into master from upstream (don’t
even think that’s possible since they are both bare).
I see. Hey, if that works for you... You could, of course, merge or
cherry-pick, or rebase your team's commits onto another copy of the FSF
(upstream) master and then send those commits: sending commits is better
than sending diffs, IMO, mostly because you get to have some metadata
and integrity protection, and because git can ensure lineage and so on.
But you could live purely with diff/patch, no question, and anywhere
between that and making full use of a VCS' powers.
Here now is what we did at Sun, mapped onto git, written as something of
a hardcopy to be more exact.
Remember, this was what we did for _all_ of Solaris. You can probably
still find docs from the OpenSolaris days describing how to do it with
Mercurial, so you can see I'm not lying. Thousands of engineers,
working on many discrete projects, with a large OS broken up into a few
consolidations (each with its own repo).
(Here the project gate is the team repo, that I think you call
master above.)
$ # on a bi-weekly (or whatever's best) basis:
$
$ git clone $foo_project_gate foo
$ cd foo
$ git remote add upstream ...
$ git fetch upstream
$ git checkout $current_master
$ new_snapshot=master-$(date +%Y-%m-%d)
$ git checkout -b $new_snapshot
$ git rebase upstream/master
$ git push origin $new_snapshot
$
$ mutt -s PROJECT FOO: Rebase onto new master branch master-$(date +%Y-%m-%d)
foo-engineers /dev/null
Then the engineers on this project do this (at their leisure):
$ old_snapshot=-mm-dd from current master branch
$ new_snapshot=-mm-dd from new master branch
$ cd $my_foo_project_clone
$ git fetch origin
$ for topic_branch in ...; do
git checkout -b ${topic_branch%-${old_snapshot}}-$new_snapshot
git rebase --onto master-$new_snapshot master-$old_snapshot
done
$
$ # Ready to pick up where I left off!
...
Eventually engineers integrate commits into the project gate:
$ # I'm ready to push to the project gate!
$
$ git checkout some_topic_branch
$
$ # Note: no -f!
$ git push origin HEAD:master-$current_snapshot
...
$ # yay
Eventually the project is ready to push its commits upstream:
$ git clone $project_gate foo
$ cd foo
$ git remote add upstream ...
$ git checkout master-$current_snapshot
$ git push upstream HEAD:master
If you're not going to be sending all local commits upstream yet then
you can do an interactive rebase, put the commits you do want to send
immediately after the upstream's HEAD commit, all the others after, and
send just those. If you do this you should create a new snapshot and
tell your team members to git rebase --onto it.
Note that we're always rebasing _new_ branches. Never old ones. The
project gate does plain rebases of those new branches. Downstreams have
to rebase --onto to recover (it works fine).
This is a very rebase-happy workflow. It keeps as-yet-not-contributed
commits on top relative to the immediate upstream of any repo. This
makes them easy to identify, and it keeps the author/date/subject
metadata. Because you rebase often, you don't lag the upstream by much.
Because they are on top it's always fast-forward merge to push --
you're always merged, with some lag, yes, but merged. And the person
doing the merging is the owner of the repo (team members, project
gatekeeper).
It's a bit more work each time you rebase than a merge-heavy workflow.
But it's also easier to contribute, and it's easier on each successive
upstream's maintainers.
(The upstream also kept snapshot branches. Doing this has many good
side effects, not the least of which is that git prune (and gc, which I
knew about) doesn't go deleting the past of each rebase.)
The only use-case I've seen where a rebase-based workflow doesn't work
Well, and now mine, which I claim is a the canonical open source use
[...]
Nah. Sun managed this for decades without a hitch, and for products
much larger than GCC. See above.
(It's true that it's difficult to sell some people on this workflow,
Re: What's cooking in git.git (Aug 2014, #01; Fri, 1)
On Fri, Aug 01, 2014 at 03:01:31PM -0700, Junio C Hamano wrote: * jk/stash-list-p (2014-07-30) 7 commits - SQUASH??? future-proof, log --cc should imply -p without being told - stash: show combined diff with stash show - stash: default listing to --cc --simplify-combined-diff - add --simplify-combined-diff option - pretty: make empty userformats truly empty - pretty: treat --format= as an empty userformat - revision: drop useless string offset when parsing --pretty Teach git stash list -p to DWIM to git stash list -p --cc, with even nicer twist to collapse combined diff from identical two parents into a regular diff. What do you want to do with this topic? I think we want to drop the stash show patch, based on the discussion we had. The first three patches are nominally prep for that final patch, but actually are things I've often wanted over the years. I'd be glad if they made it in separately, but there were some compatibility questions. As clever as I find the --simplify-combined-diff patch, I think we came to the conclusion that --first-parent is probably the reasonable choice. It matches stash show, and it's simple and obvious. Do we just want a patch to specify --first-parent to stash-log? That would make -p just work. The only downside is that there isn't a good way to turn it off. Is it enough to say if you want to do something clever, use git-log? Or do we want to scrap the whole thing and try to update the documentation to make it more clear why -p by itself doesn't do anything? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Odd results with git-describe
Hi, with rust's git repo -- located at https://github.com/rust-lang/rust.git -- we see odd results from git-describe for the latest tag. $ git rev-parse HEAD 795f6ae829ab1bfd72394a5da9096e2717ec0f62 $ git describe --debug searching to describe HEAD finished search at 7140d7c52bdf55daf0b978a19706d20c3bf7ee92 annotated 3627 0.10 annotated 5929 0.9 annotated 8581 0.8 annotated 11686 0.7 annotated 14540 0.6 annotated 15230 0.11.0 traversed 15232 commits 0.10-3627-g795f6ae Somehow it reports that 0.11.0 is missing tons of commits that are in HEAD. rev-list disagrees with this: $ git rev-list 0.11.0.. | wc -l 1155 The `SEARCH STRATEGY` section of the manpage says that the commit numbers for describe and rev-list should match, which is not the case here. Björn -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/config.txt: change pull.rebase description in favor of safer 'preserve' option.
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Previous description implicitly favored 'true' value for pull.rebase and pull.branch.rebase options, I do not share that impression. It is true that 'true' is described first before 'preserve', which can be argued that it is being implicitly favoured, but we have to pick one over the other when describing two things, so I do not see it as a strong preference. I didn't say it's a strong preference, and I consider my patch to be minor documenation improvement that may save from major trouble. I still suggest to peek 'preserve' over 'true' in the description, an opposite to what is there right now. I believe it'd help me to avoid the mistake I did setting pull.merge to true, by stopping reading when I saw that pull.rebase should be set to 'true' to rebase instead of merge. Couldn't you please just read the new description and tell what's wrong with it? Besides, it puts less favor on one mode over another than the original. Am I right that 'true' for pull.rebase historically precedes 'preserve'? This is the only cause of current wording in documentation I can imagine. when for some workflows 'preserve' is the right choice, and for others it hardly makes any difference. Therefore, 'preserve' should be preferred in general, unless the user knows exactly what she is doing. I doubt we saw any such conclusion in the recent thread that discussed this, other than your repeating that statement---and I've learned to tell repeated voices and chorus apart over time ;-). I've repeated request to tell me if somebody has any evidence 'preserve' would break their workflow, and nobody provided one. I even hoped you would say something, but it didn't happen till I suggested the patch ;-) On the other hand, I did suffer from 'true' setting, and that seems to be more common, as was shown by the problem of another git user soon after I explained mine. Moreover, the order of suffer is worse with 'true' than it ever could be with 'preserve', see below. One approach is more suitable for some workflows while being inappropriate for others and that goes for both variants. A downside of flattening is that it gives a wrong result when the user wants to keep merges. Two downsides of preserving are that it gives a wrong result when the user wants to flatten, and it tends to be slower. I understand this, but it does look like most of those who care about entirely flat history don't do merges in the first place. At least I didn't do it for my simple changes for 'git', so I was not aware how dangerous the 'true' setting for pull.rebase I was using is for my own workflows, when I started using git internally. I'd also argue that if they do the merges themselves, they'd better do flattening themselves as well (by git rebase), rather than (ab)using pull.rebase feature which primary use is avoiding *automatic* merges caused by pull(s). During that discussion, you seemed to assume that those who want a flattened end result would never merge locally; I do not think that is true. Having your own merges on a branch that you would want to rebase to flatten is not unusual. I didn't assume, I rather tried to figure it. However, I didn't hear from anybody who do it regularly, and I waited for 2 weeks, so I decided that even if there are any, they are in minority, or maybe are experienced enough not to care. But that even is not my main argument. Look, 'preserve' preserves things. If one doesn't like the result, he can just rebase, or set pull.merge to true and repeat the pull, or just pull --rebase. Simple. You are still inside the same set of commands and concepts. On the other hand, recovering from unneeded flattening is much more difficult, especially for novice, as it requres familarity with a bunch of new concepts behind git reset. You may employ a workflow to build on separate topic branches while developing, merge the topics among them that are ready into your local 'master' to be used personally, and after using it from your local 'master' for a while to make sure it is solid, you may decide to make it final by publishing it to the outside world, and at that point you would want to flatten the history on top of the latest upstream before pushing. I'm not proposing to remove any functionality, do I? It's just that 'preserve' is safer in general and easier to deal with when result doesn't fit. Once again, recovering from unexpected flattening is more difficult. BTW, even in this workflow, forgetting to pull after merge and before push may still lead to the same result that 'preserve' has (no changes upstream, so merge will be simply pushed), so setting pull.merge to true doesn't eliminate the problem entirely even in this case. I think the right advice for such workflow would then be to flatten your history locally using git rebase explicitly, before trying to push something upstream, that in turn would make this
Re: rebase flattens history when it shouldn't?
Jonathan Nieder jrnie...@gmail.com writes: Hi Sergei, Sergei Organov wrote: --C-- / \ / M topic,HEAD / / A---B master shouldn't $ git rebase master be a no-op here? [...] I'd expect --force-rebase to be required for this to happen: -f, --force-rebase Force the rebase even if the current branch is a descendant of the commit you are rebasing onto. Normally non-interactive rebase will exit with the message Current branch is up to date in such a situation. [...] Do you think it's worth fixing? Thanks for a clear report. After a successful 'git rebase master', the current branch is always a linear string of patches on top of 'master'. Is this documented? Except implicitly by the: -p, --preserve-merges Instead of ignoring merges, try to recreate them. ?? Anyway, why such a requirement, and is it actually enforced by tests? The already up to date behavior when -f is not passed is in a certain sense an optimization --- it is about git noticing that 'git rebase' wouldn't have anything to do (except for touching timestamps) and therefore doing nothing. Maybe, but I'd argue it's rather sane behavior to do no rebase when new rebase point is the same as original in general. I.e., when current branch is a descendant of the commit you are rebasing onto, as documentation says. So I don't think requiring -f for this case would be an improvement. I still do, as it will match documentation, that in turn looks reasonable. I do agree that the documentation is misleading. If the problem is in documentation, it's not only misleading, it's formally wrong. Any ideas for wording that could make it clearer? Well, if it's indeed documentation, how about this: diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 2a93c64..62dac31 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -316,10 +316,9 @@ which makes little sense. -f:: --force-rebase:: - Force the rebase even if the current branch is a descendant - of the commit you are rebasing onto. Normally non-interactive rebase will - exit with the message Current branch is up to date in such a - situation. + Force the rebase even if the result will only change commit + timestamps. Normally non-interactive rebase will exit with the + message Current branch is up to date in such a situation. Incompatible with the --interactive option. + You may find this (or --no-ff with an interactive rebase) helpful after BTW, why Incompatible with the --interactive option.? Isn't force assumed by --interactive, functionally? -- Sergey. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi
Can I talk to you, I will be very glad if you permit me to talk to you for an important deal. Regards, KM. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 0/8] Rewrite `git_config()` using config-set API
[Patch v8]: git_die_config now allows custom error messages.
new tests are now not too reliant on specific strings. Diff
between v7 v8 is appended at the bottom. Thanks to Junio
Matthieu for their suggestions.
[Patch v7]: style nit corrected. (1/8) is Matthieu's translation patch.
git_die_config_linenr() helper function added. Diff between v6
and v7 appended for review.
[Patch v6]: Added _() to error messages.
Diff between v6 and v4 at the bottom.
[PATCH v5]: New patch added (3/7). git_config() now returns void.
[PATCH v4]: One style nit corrected, also added key to error messages.
[PATCH V3]:All the suggestions in [3] applied. Built on top of [1].
[PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows
correct parsing order. Reordered the patches. Removed xfuncname patch
as it was unnecssary.
This series builds on the top of topic[1] in the mailing list with name
git config cache special querying API utilizing the cache.
This series aims to do these three things,
* Use the config-set API to rewrite git_config().
* Solve any legacy bugs in the previous system while at it.
* To be feature complete compared to the previous git_config() implementation,
which I think it is now. (added the line number and file name info just
for
completeness)
Also, I haven't yet checked the exact improvements but still as a teaser,
git status now only rereads the configuration files twice instead of four
times.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/254286
[2]: http://thread.gmane.org/gmane.comp.version-control.git/254101
[3]: http://thread.gmane.org/gmane.comp.version-control.git/254211
Matthieu Moy (1):
config.c: mark error and warnings strings for translation
Tanay Abhra (7):
config.c: fix accuracy of line number in errors
add line number and file name info to `config_set`
change `git_config()` return value to void
config: add `git_die_config()` to the config-set API
rewrite git_config() to use the config-set API
add a test for semantic errors in config files
add tests for `git_config_get_string_const()`
Documentation/technical/api-config.txt | 13 +++
branch.c | 5 +-
cache.h| 34 +++-
config.c | 152 +++--
t/t1308-config-set.sh | 21 +
t/t4055-diff-context.sh| 2 +-
test-config.c | 10 +++
7 files changed, 207 insertions(+), 30 deletions(-)
--
1.9.0.GIT
-- 8 --
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index d7d0cf9..0d8b99b 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,10 +155,11 @@ as well as retrieval for the queried variable, including:
Similar to `git_config_get_string`, but expands `~` or `~user` into
the user's home directory when found at the beginning of the path.
-`void git_die_config(const char *key)`::
+`git_die_config(const char *key, const char *err, ...)`::
- Dies printing the line number and the file name of the highest
- priority value for the configuration variable `key`.
+ First prints the error message specified by the caller in `err` and then
+ dies printing the line number and the file name of the highest priority
+ value for the configuration variable `key`.
`void git_die_config_linenr(const char *key, const char *filename, int
linenr)`::
diff --git a/cache.h b/cache.h
index ff17889..2693a37 100644
--- a/cache.h
+++ b/cache.h
@@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int
*dest);
extern int git_config_get_bool_or_int(const char *key, int *is_bool, int
*dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
-extern void git_die_config(const char *key);
-extern void git_die_config_linenr(const char *key, const char *filename, int
linenr);
+
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
+
+extern NORETURN void git_die_config(const char *key, const char *err, ...)
__attribute__((format(printf, 2, 3)));
+extern NORETURN void git_die_config_linenr(const char *key, const char
*filename, int linenr);
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index cf9124f..427850a 100644
--- a/config.c
+++ b/config.c
@@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
-struct key_value_info {
- const char *filename;
- int linenr;
-};
-
static void git_config_raw(config_fn_t fn, void *data)
{
if (git_config_with_options(fn, data, NULL, 1) 0)
@@ -1326,8 +1321,8 @@ static int
[PATCH v8 4/8] change `git_config()` return value to void
Currently `git_config()` returns an integer signifying an error code.
During rewrites of the function most of the code was shifted to
`git_config_with_options()`. `git_config_with_options()` normally
returns positive values if its `config_source` parameter is set as NULL,
as most errors are fatal, and non-fatal potential errors are guarded
by if statements that are entered only when no error is possible.
Still a negative value can be returned in case of race condition between
`access_or_die()` `git_config_from_file()`. Also, all callers of
`git_config()` ignore the return value except for one case in branch.c.
Change `git_config()` return value to void and make it die if it receives
a negative value from `git_config_with_options()`.
Original-patch-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
branch.c | 5 +
cache.h | 2 +-
config.c | 16 ++--
3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/branch.c b/branch.c
index 46e8aa8..735767d 100644
--- a/branch.c
+++ b/branch.c
@@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char
*branch_name)
strbuf_addf(name, branch.%s.description, branch_name);
cb.config_name = name.buf;
cb.value = NULL;
- if (git_config(read_branch_desc_cb, cb) 0) {
- strbuf_release(name);
- return -1;
- }
+ git_config(read_branch_desc_cb, cb);
if (cb.value)
strbuf_addstr(buf, cb.value);
strbuf_release(name);
diff --git a/cache.h b/cache.h
index 0b1bdfd..f11ce41 100644
--- a/cache.h
+++ b/cache.h
@@ -1294,7 +1294,7 @@ extern int git_config_from_buf(config_fn_t fn, const char
*name,
const char *buf, size_t len, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern int git_config(config_fn_t fn, void *);
+extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
int respect_includes);
diff --git a/config.c b/config.c
index e4d745e..4cefa25 100644
--- a/config.c
+++ b/config.c
@@ -1230,9 +1230,21 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
-int git_config(config_fn_t fn, void *data)
+void git_config(config_fn_t fn, void *data)
{
- return git_config_with_options(fn, data, NULL, 1);
+ if (git_config_with_options(fn, data, NULL, 1) 0)
+ /*
+* git_config_with_options() normally returns only
+* positive values, as most errors are fatal, and
+* non-fatal potential errors are guarded by if
+* statements that are entered only when no error is
+* possible.
+*
+* If we ever encounter a non-fatal error, it means
+* something went really wrong and we should stop
+* immediately.
+*/
+ die(_(unknown error occured while reading the configuration
files));
}
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 2/8] config.c: fix accuracy of line number in errors
From: Matthieu Moy matthieu@imag.fr
If a callback returns a negative value to `git_config*()` family,
they call `die()` while printing the line number and the file name.
Currently the printed line number is off by one, thus printing the
wrong line number.
Make `linenr` point to the line we just parsed during the call
to callback to get accurate line number in error messages.
Commit-message-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Tanay Abhra tanay...@gmail.com
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
config.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 34940fd..bb4629e 100644
--- a/config.c
+++ b/config.c
@@ -244,6 +244,7 @@ static int get_next_char(void)
cf-linenr++;
if (c == EOF) {
cf-eof = 1;
+ cf-linenr++;
c = '\n';
}
return c;
@@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
{
int c;
char *value;
+ int ret;
/* Get the full name */
for (;;) {
@@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
if (!value)
return -1;
}
- return fn(name-buf, value, data);
+ /*
+* We already consumed the \n, but we need linenr to point to
+* the line we just parsed during the call to fn to get
+* accurate line number in error messages.
+*/
+ cf-linenr--;
+ ret = fn(name-buf, value, data);
+ cf-linenr++;
+ return ret;
}
static int get_extended_base_var(struct strbuf *name, int c)
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 3/8] add line number and file name info to `config_set`
Store file name and line number for each key-value pair in the cache
during parsing of the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 5 +
config.c | 16 ++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 7292aef..0b1bdfd 100644
--- a/cache.h
+++ b/cache.h
@@ -1383,6 +1383,11 @@ extern int git_config_get_bool_or_int(const char *key,
int *is_bool, int *dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
+
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index bb4629e..e4d745e 100644
--- a/config.c
+++ b/config.c
@@ -1260,6 +1260,9 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
static int configset_add_value(struct config_set *cs, const char *key, const
char *value)
{
struct config_set_element *e;
+ struct string_list_item *si;
+ struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
+
e = configset_find_element(cs, key);
/*
* Since the keys are being fed by git_config*() callback mechanism,
they
@@ -1272,7 +1275,16 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
string_list_init(e-value_list, 1);
hashmap_add(cs-config_hash, e);
}
- string_list_append_nodup(e-value_list, value ? xstrdup(value) : NULL);
+ si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+ if (cf) {
+ kv_info-filename = strintern(cf-name);
+ kv_info-linenr = cf-linenr;
+ } else {
+ /* for values read from `git_config_from_parameters()` */
+ kv_info-filename = NULL;
+ kv_info-linenr = -1;
+ }
+ si-util = kv_info;
return 0;
}
@@ -1299,7 +1311,7 @@ void git_configset_clear(struct config_set *cs)
hashmap_iter_init(cs-config_hash, iter);
while ((entry = hashmap_iter_next(iter))) {
free(entry-key);
- string_list_clear(entry-value_list, 0);
+ string_list_clear(entry-value_list, 1);
}
hashmap_free(cs-config_hash, 1);
cs-hash_initialized = 0;
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 1/8] config.c: mark error and warnings strings for translation
From: Matthieu Moy matthieu@imag.fr
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
config.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/config.c b/config.c
index a191328..34940fd 100644
--- a/config.c
+++ b/config.c
@@ -457,9 +457,9 @@ static int git_parse_source(config_fn_t fn, void *data)
break;
}
if (cf-die_on_error)
- die(bad config file line %d in %s, cf-linenr, cf-name);
+ die(_(bad config file line %d in %s), cf-linenr, cf-name);
else
- return error(bad config file line %d in %s, cf-linenr,
cf-name);
+ return error(_(bad config file line %d in %s), cf-linenr,
cf-name);
}
static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -575,9 +575,9 @@ static void die_bad_number(const char *name, const char
*value)
value = ;
if (cf cf-name)
- die(bad numeric config value '%s' for '%s' in %s: %s,
+ die(_(bad numeric config value '%s' for '%s' in %s: %s),
value, name, cf-name, reason);
- die(bad numeric config value '%s' for '%s': %s, value, name, reason);
+ die(_(bad numeric config value '%s' for '%s': %s), value, name,
reason);
}
int git_config_int(const char *name, const char *value)
@@ -662,7 +662,7 @@ int git_config_pathname(const char **dest, const char *var,
const char *value)
return config_error_nonbool(var);
*dest = expand_user_path(value);
if (!*dest)
- die(Failed to expand user dir in: '%s', value);
+ die(_(failed to expand user dir in: '%s'), value);
return 0;
}
@@ -740,7 +740,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
zlib_compression_level = level;
zlib_compression_seen = 1;
return 0;
@@ -751,7 +751,7 @@ static int git_default_core_config(const char *var, const
char *value)
if (level == -1)
level = Z_DEFAULT_COMPRESSION;
else if (level 0 || level Z_BEST_COMPRESSION)
- die(bad zlib compression level %d, level);
+ die(_(bad zlib compression level %d), level);
core_compression_level = level;
core_compression_seen = 1;
if (!zlib_compression_seen)
@@ -873,7 +873,7 @@ static int git_default_core_config(const char *var, const
char *value)
else if (!strcmp(value, link))
object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
else
- die(Invalid mode for object creation: %s, value);
+ die(_(invalid mode for object creation: %s), value);
return 0;
}
@@ -1173,7 +1173,7 @@ int git_config_early(config_fn_t fn, void *data, const
char *repo_config)
switch (git_config_from_parameters(fn, data)) {
case -1: /* error */
- die(unable to parse command-line config);
+ die(_(unable to parse command-line config));
break;
case 0: /* found nothing */
break;
@@ -1514,7 +1514,7 @@ static int store_aux(const char *key, const char *value,
void *cb)
case KEY_SEEN:
if (matches(key, value)) {
if (store.seen == 1 store.multi_replace == 0) {
- warning(%s has multiple values, key);
+ warning(_(%s has multiple values), key);
}
ALLOC_GROW(store.offset, store.seen + 1,
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 5/8] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name
of the highest priority value for the configuration variable `key`. A custom
error message is also printed before dying, specified by the caller, which can
be skipped if `err` argument is set to NULL.
It has usage in non-callback based config value retrieval where we can
raise an error and die if there is a semantic error.
For example,
if (!git_config_get_value(key, value)){
if (!strcmp(value, foo))
git_config_die(key, value: `%s` is illegal, value);
else
/* do work */
}
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Documentation/technical/api-config.txt | 13
cache.h| 3 +++
config.c | 39 --
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/api-config.txt
b/Documentation/technical/api-config.txt
index 21f280c..0d8b99b 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,6 +155,19 @@ as well as retrieval for the queried variable, including:
Similar to `git_config_get_string`, but expands `~` or `~user` into
the user's home directory when found at the beginning of the path.
+`git_die_config(const char *key, const char *err, ...)`::
+
+ First prints the error message specified by the caller in `err` and then
+ dies printing the line number and the file name of the highest priority
+ value for the configuration variable `key`.
+
+`void git_die_config_linenr(const char *key, const char *filename, int
linenr)`::
+
+ Helper function which formats the die error message according to the
+ parameters entered. Used by `git_die_config()`. It can be used by
callers
+ handling `git_config_get_value_multi()` to print the correct error
message
+ for the desired value.
+
See test-config.c for usage examples.
Value Parsing Helpers
diff --git a/cache.h b/cache.h
index f11ce41..89a0d51 100644
--- a/cache.h
+++ b/cache.h
@@ -1388,6 +1388,9 @@ struct key_value_info {
int linenr;
};
+extern NORETURN void git_die_config(const char *key, const char *err, ...)
__attribute__((format(printf, 2, 3)));
+extern NORETURN void git_die_config_linenr(const char *key, const char
*filename, int linenr);
+
extern int committer_ident_sufficiently_given(void);
extern int author_ident_sufficiently_given(void);
diff --git a/config.c b/config.c
index 4cefa25..5ae9ab0 100644
--- a/config.c
+++ b/config.c
@@ -1469,8 +1469,12 @@ const struct string_list
*git_config_get_value_multi(const char *key)
int git_config_get_string_const(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_string_const(the_config_set, key, dest);
+ ret = git_configset_get_string_const(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key, NULL);
+ return ret;
}
int git_config_get_string(const char *key, char **dest)
@@ -1511,8 +1515,39 @@ int git_config_get_maybe_bool(const char *key, int *dest)
int git_config_get_pathname(const char *key, const char **dest)
{
+ int ret;
git_config_check_init();
- return git_configset_get_pathname(the_config_set, key, dest);
+ ret = git_configset_get_pathname(the_config_set, key, dest);
+ if (ret 0)
+ git_die_config(key, NULL);
+ return ret;
+}
+
+NORETURN
+void git_die_config_linenr(const char *key, const char *filename, int linenr)
+{
+ if (!filename)
+ die(_(unable to parse '%s' from command-line config), key);
+ else
+ die(_(bad config variable '%s' in file '%s' at line %d),
+ key, filename, linenr);
+}
+
+NORETURN __attribute__((format(printf, 2, 3)))
+void git_die_config(const char *key, const char *err, ...)
+{
+ const struct string_list *values;
+ struct key_value_info *kv_info;
+
+ if (err) {
+ va_list params;
+ va_start(params, err);
+ vreportf(error: , err, params);
+ va_end(params);
+ }
+ values = git_config_get_value_multi(key);
+ kv_info = values-items[values-nr - 1].util;
+ git_die_config_linenr(key, kv_info-filename, kv_info-linenr);
}
/*
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 6/8] rewrite git_config() to use the config-set API
Of all the functions in `git_config*()` family, `git_config()` has the
most invocations in the whole code base. Each `git_config()` invocation
causes config file rereads which can be avoided using the config-set API.
Use the config-set API to rewrite `git_config()` to use the config caching
layer to avoid config file rereads on each invocation during a git process
lifetime. First invocation constructs the cache, and after that for each
successive invocation, `git_config()` feeds values from the config cache
instead of rereading the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 24 +++
config.c| 51 +
t/t4055-diff-context.sh | 2 +-
3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index 89a0d51..2693a37 100644
--- a/cache.h
+++ b/cache.h
@@ -8,6 +8,7 @@
#include gettext.h
#include convert.h
#include trace.h
+#include string-list.h
#include SHA1_HEADER
#ifndef git_SHA_CTX
@@ -1351,9 +1352,32 @@ extern int parse_config_key(const char *var,
const char **subsection, int *subsection_len,
const char **key);
+struct config_set_element {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list value_list;
+};
+
+struct configset_list_item {
+ struct config_set_element *e;
+ int value_index;
+};
+
+/*
+ * the contents of the list are ordered according to their
+ * position in the config files and order of parsing the files.
+ * (i.e. key-value pair at the last position of .git/config will
+ * be at the last item of the list)
+ */
+struct configset_list {
+ struct configset_list_item *items;
+ unsigned int nr, alloc;
+};
+
struct config_set {
struct hashmap config_hash;
int hash_initialized;
+ struct configset_list list;
};
extern void git_configset_init(struct config_set *cs);
diff --git a/config.c b/config.c
index 5ae9ab0..427850a 100644
--- a/config.c
+++ b/config.c
@@ -35,12 +35,6 @@ struct config_source {
long (*do_ftell)(struct config_source *c);
};
-struct config_set_element {
- struct hashmap_entry ent;
- char *key;
- struct string_list value_list;
-};
-
static struct config_source *cf;
static int zlib_compression_seen;
@@ -1230,7 +1224,7 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
-void git_config(config_fn_t fn, void *data)
+static void git_config_raw(config_fn_t fn, void *data)
{
if (git_config_with_options(fn, data, NULL, 1) 0)
/*
@@ -1247,6 +1241,33 @@ void git_config(config_fn_t fn, void *data)
die(_(unknown error occured while reading the configuration
files));
}
+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+ int i, value_index;
+ struct string_list *values;
+ struct config_set_element *entry;
+ struct configset_list *list = cs-list;
+ struct key_value_info *kv_info;
+
+ for (i = 0; i list-nr; i++) {
+ entry = list-items[i].e;
+ value_index = list-items[i].value_index;
+ values = entry-value_list;
+ if (fn(entry-key, values-items[value_index].string, data)
0) {
+ kv_info = values-items[value_index].util;
+ git_die_config_linenr(entry-key, kv_info-filename,
kv_info-linenr);
+ }
+ }
+}
+
+static void git_config_check_init(void);
+
+void git_config(config_fn_t fn, void *data)
+{
+ git_config_check_init();
+ configset_iter(the_config_set, fn, data);
+}
+
static struct config_set_element *configset_find_element(struct config_set
*cs, const char *key)
{
struct config_set_element k;
@@ -1273,6 +1294,7 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
{
struct config_set_element *e;
struct string_list_item *si;
+ struct configset_list_item *l_item;
struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
e = configset_find_element(cs, key);
@@ -1288,6 +1310,12 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+
+ ALLOC_GROW(cs-list.items, cs-list.nr + 1, cs-list.alloc);
+ l_item = cs-list.items[cs-list.nr++];
+ l_item-e = e;
+ l_item-value_index = e-value_list.nr - 1;
+
if (cf) {
kv_info-filename = strintern(cf-name);
kv_info-linenr = cf-linenr;
@@ -1311,6 +1339,9 @@ void git_configset_init(struct config_set *cs)
{
hashmap_init(cs-config_hash, (hashmap_cmp_fn)config_set_element_cmp,
0);
cs-hash_initialized = 1;
+ cs-list.nr = 0;
+
[PATCH v8 8/8] add tests for `git_config_get_string_const()`
Add tests for `git_config_get_string_const()`, check whether it
dies printing the line number and the file name if a NULL
value is retrieved for the given key.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
t/t1308-config-set.sh | 10 ++
test-config.c | 10 ++
2 files changed, 20 insertions(+)
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index 5ea0aa4..ecc757a 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -119,6 +119,16 @@ test_expect_success 'find integer value for a key' '
check_config get_int lamb.chop 65
'
+test_expect_success 'find string value for a key' '
+ check_config get_string case.baz hask
+ check_config expect_code 1 get_string case.ba Value not found for
\case.ba\
+'
+
+test_expect_success 'check line error when NULL string is queried' '
+ test_expect_code 128 test-config get_string case.foo 2result
+ grep line 7.*.git/config\|.git/config.*line 7 result
+'
+
test_expect_success 'find integer if value is non parse-able' '
check_config expect_code 128 get_int lamb.head
'
diff --git a/test-config.c b/test-config.c
index 9dd1b22..6a77552 100644
--- a/test-config.c
+++ b/test-config.c
@@ -16,6 +16,8 @@
*
* get_bool - print bool value for the entered key or die
*
+ * get_string - print string value for the entered key or die
+ *
* configset_get_value - returns value with the highest priority for the
entered key
* from a config_set constructed from files entered as
arguments.
*
@@ -84,6 +86,14 @@ int main(int argc, char **argv)
printf(Value not found for \%s\\n, argv[2]);
goto exit1;
}
+ } else if (argc == 3 !strcmp(argv[1], get_string)) {
+ if (!git_config_get_string_const(argv[2], v)) {
+ printf(%s\n, v);
+ goto exit0;
+ } else {
+ printf(Value not found for \%s\\n, argv[2]);
+ goto exit1;
+ }
} else if (!strcmp(argv[1], configset_get_value)) {
for (i = 3; i argc; i++) {
int err;
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v8 7/8] add a test for semantic errors in config files
Semantic errors (for example, for alias.* variables NULL values are not allowed) in configuration files cause a die printing the line number and file name of the offending value. Add a test documenting that such errors cause a die printing the accurate line number and file name. Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1308-config-set.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..5ea0aa4 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -197,4 +197,15 @@ test_expect_success 'proper error on error in custom config files' ' test_cmp expect actual ' +test_expect_success 'check line errors for malformed values' ' + mv .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + cat .git/config -\EOF + [alias] + br + EOF + test_expect_code 128 git br 2result + grep line 2.*.git/config\|.git/config.*line 2 result +' + test_done -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git detects changes after new branch created and switched even though files have not been changed
Running on Windows with latest version of Git. 1.9.4. Also had this trouble with 1.8.2 but upgraded hoping it would fix the problem. When I do a reset on several files, it says they were successfully reverted they still show up as modified when I do a git status. In this case it's just a newline character at the end of the file. I also tried a reset -- hard and they files still show up! What the heck! Help please. In trying to reproduce this, I re-cloned the git repo from the server, which was in the exact clean state before I started last time. No files have been modified after cloning, all are identical. Then, I created a new branch (totally new, not tracking a remote branch). Viola - I now have these files show up in my git status as modified. Please note that these are files that have been tracked for several previous commits. Just to clarify - the only command I ran to make this happen is: git checkout -b newbranch And GitGui shows the files as identical even when compare whitespace is turned on. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] http-backend.c: replace `git_config()` with `git_config_get_bool()` family
On 8/5/2014 12:29 AM, Eric Sunshine wrote:
On Mon, Aug 4, 2014 at 2:33 PM, Tanay Abhra tanay...@gmail.com wrote:
Use `git_config_get_bool()` family instead of `git_config()` to take
advantage of
the config-set API which provides a cleaner control flow.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
http-backend.c | 31 ---
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index 80790bb..106ca6b 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -219,29 +219,22 @@ static void get_idx_file(char *name)
send_local_file(application/x-git-packed-objects-toc, name);
}
-static int http_config(const char *var, const char *value, void *cb)
+static void http_config(void)
{
- const char *p;
+ int i, value = 0;
+ struct strbuf var = STRBUF_INIT;
- if (!strcmp(var, http.getanyfile)) {
- getanyfile = git_config_bool(var, value);
- return 0;
- }
+ git_config_get_bool(http.getanyfile, getanyfile);
- if (skip_prefix(var, http., p)) {
- int i;
-
- for (i = 0; i ARRAY_SIZE(rpc_service); i++) {
- struct rpc_service *svc = rpc_service[i];
- if (!strcmp(p, svc-config_name)) {
- svc-enabled = git_config_bool(var, value);
- return 0;
- }
- }
+ for (i = 0; i ARRAY_SIZE(rpc_service); i++) {
+ struct rpc_service *svc = rpc_service[i];
+ strbuf_addf(var, http.%s, svc-config_name);
+ if (!git_config_get_bool(var.buf, value))
+ svc-enabled = value;
+ strbuf_reset(var);
}
There is a behavior change here. The original code set svc-enabled to
the first matching rpc_service[] entry, whereas the new code sets it
to the last matching entry. Is this change intentional?
I was preparing the reroll and I saw that I had missed your mail.
I think that I haven't changed the behaviour, the original one is
written in callback form so it has to go through the array every time for each
new value.
When there are multiple entries for a service say,
http.receivepack = 1
http.receivepack = 0
the old code would have at overwritten the previous entry with the new value.
The new code just populates the whole array of `rpc_service` in one go, choosing
the last matching value for each entry. For reviewing purpose the original
array is
this,
struct rpc_service {
const char *name;
const char *config_name;
signed enabled : 2;
};
static struct rpc_service rpc_service[] = {
{ upload-pack, uploadpack, 1 },
{ receive-pack, receivepack, -1 },
};
What do you think, am I interpreting it wrong? Thanks.
- /* we are not interested in parsing any other configuration here */
- return 0;
+ strbuf_release(var);
}
static struct rpc_service *select_service(const char *name)
@@ -627,7 +620,7 @@ int main(int argc, char **argv)
access(git-daemon-export-ok, F_OK) )
not_found(Repository not exported: '%s', dir);
- git_config(http_config, NULL);
+ http_config();
cmd-imp(cmd_arg);
return 0;
}
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: rebase flattens history when it shouldn't?
On 23.07.2014 21:33, Sergei Organov wrote: What actually bothers me is the unfortunate consequence that git pull is not always a no-op when nothing was changed at the origin since the last git pull. THIS is really surprising and probably should better be fixed. Requiring -f is just one (obvious) way to fix this. That would invalidate the simple rule that git pull is equivalent to git fetch + git rebase. git rebase depends on both branches it operates on, not just one. The same goes for git merge, I assume it is just a coincidence that git merge does have this characteristic you now expect both to have. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API
Tanay Abhra tanay...@gmail.com writes:
--- a/cache.h
+++ b/cache.h
@@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int
*dest);
[...]
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
[...]
diff --git a/config.c b/config.c
index cf9124f..427850a 100644
--- a/config.c
+++ b/config.c
@@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
-struct key_value_info {
- const char *filename;
- int linenr;
-};
-
Why is this needed? Are you now using key_value_info outside config.c?
Or is it a leftover from a previous experiment?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 8/8] add tests for `git_config_get_string_const()`
Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'find string value for a key' ' + check_config get_string case.baz hask + check_config expect_code 1 get_string case.ba Value not found for \case.ba\ +' + +test_expect_success 'check line error when NULL string is queried' ' + test_expect_code 128 test-config get_string case.foo 2result + grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
Philip Oakley wrote: From: Mike Stump mikest...@comcast.net Sent: Friday, August 01, 2014 11:24 PM On Aug 1, 2014, at 12:01 PM, Jakub Narębski jna...@gmail.com wrote: It can work in Subversion because Subversion stores information about what was merged in (and this includes cherry-picks, or whatever it is named in svn) in svn:mergeinfo property. Git does not track what was merged in, instead it represent the history as the graph of revisions, and tracks merges (by storing that it came from two or more commits) and not merged-in information. So, as a dumb user that just wants it to work, I am unsympathetic to the `but software is hard’ excuse. I am aware that some bugs are harder to fix than others. svn took a long time to fix this bug, but they did. I can wait, the only question is, will it be a week, a month, a year, or a decade. Here Git and Subversion went in different directions, and use different mechanisms (merge tracking vs merged-on tracking). Both have their advantages and disadvantages. git-merge (in the most usual case) depends only on three revisions: the revision you merge into (current branch, ours), the revision you are merging (merged branch, theirs), and merge base (common ancestor). We could have another merge strategy that examines contents of revisions to handle cherry-picks and reverts... but it would be more complicated, and much slower. When merging Git uses only what is being merged and its common ancestor (3-point merge). It is simple, and simple works!!! I gave a solution for git using branches and it works just fine. It retains the simple 3-point merge as well. It works for this simple case, but I think it has unfortunate potential to go silently wrong. Also, it prevents fully removing (commits, not only refs) the branch you cherry-picked from. The commit you cherry picked may no longer be (or may no longer should be) in the repository. At the moment there is no formal way for Git to record within the commit metadata the inclusion of the cherry-picked diff (the 'merge' of the fix). Thinking out of the box, the issue is that the commit parents list does not have a formal mechanism to allow the recording that the 'merged' change was the patch change from a specific commit fom somewhere else (which may be missing from the local repo). Perhaps it needs a style of merging-rebase where a second (last) parent is added but it isn't the straight sha1, but says 'patch-sha1', such that readers with the capability could check if that sha1 history is present locally, and if so if it's correct, so that you can now 'track' your fixes between releases, and (hopefully) older Gits don't barf on that extra 'fake' parent. Somehow I suspect that older Git's would barf.. (not enough time to create and test such a fake commit). Sometime ago there was long discussion about adding 'weak' references to commit object header. Beside the problem of backward compatibility, there was also the problem of semantics of said reference - what does it mean? It should work as well for cherry-picks, for interactive rebase (maybe?), and for reverts (which are also a problem). Also, this could be avoided by using feature branches and merging instead of committing to one branch and cherry-picking to other branches. Also, git-rerere is your friend... sometimes. Have you tried git-imerge? No, not yet. I’m not as interested in using it, as I would like git itself to just work. Maybe this command would make it into git proper, though probably not written in Python (there was once merge strategy written in Python, but currently git does not depend on Python). -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 8/8] add tests for `git_config_get_string_const()`
On 8/6/2014 9:02 PM, Matthieu Moy wrote: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'find string value for a key' ' +check_config get_string case.baz hask +check_config expect_code 1 get_string case.ba Value not found for \case.ba\ +' + +test_expect_success 'check line error when NULL string is queried' ' +test_expect_code 128 test-config get_string case.foo 2result +grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). Oh, and I was searching t/README for a hint. Maybe we should add a line there to hint future readers. Thanks. :) I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? I think he meant we must search for the relevant snippets instead of the whole string. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 3/8] add line number and file name info to `config_set`
On 06/08/14 15:53, Tanay Abhra wrote:
Store file name and line number for each key-value pair in the cache
during parsing of the configuration files.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 5 +
config.c | 16 ++--
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 7292aef..0b1bdfd 100644
--- a/cache.h
+++ b/cache.h
@@ -1383,6 +1383,11 @@ extern int git_config_get_bool_or_int(const char *key,
int *is_bool, int *dest);
extern int git_config_get_maybe_bool(const char *key, int *dest);
extern int git_config_get_pathname(const char *key, const char **dest);
+struct key_value_info {
+ const char *filename;
+ int linenr;
+};
+
Hmm, why was this moved here? As far as I can tell, it is
(still) not needed outside of config.c. What am I missing?
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/11] http-backend.c: replace `git_config()` with `git_config_get_bool()` family
Tanay Abhra tanay...@gmail.com writes:
On 8/5/2014 12:29 AM, Eric Sunshine wrote:
On Mon, Aug 4, 2014 at 2:33 PM, Tanay Abhra tanay...@gmail.com wrote:
- if (skip_prefix(var, http., p)) {
- int i;
-
- for (i = 0; i ARRAY_SIZE(rpc_service); i++) {
- struct rpc_service *svc = rpc_service[i];
- if (!strcmp(p, svc-config_name)) {
- svc-enabled = git_config_bool(var, value);
- return 0;
- }
- }
+ for (i = 0; i ARRAY_SIZE(rpc_service); i++) {
+ struct rpc_service *svc = rpc_service[i];
+ strbuf_addf(var, http.%s, svc-config_name);
+ if (!git_config_get_bool(var.buf, value))
+ svc-enabled = value;
+ strbuf_reset(var);
}
There is a behavior change here. The original code set svc-enabled to
the first matching rpc_service[] entry, whereas the new code sets it
to the last matching entry. Is this change intentional?
I was preparing the reroll and I saw that I had missed your mail.
I think that I haven't changed the behaviour, the original one is
written in callback form so it has to go through the array every time for each
new value.
When there are multiple entries for a service say,
http.receivepack = 1
http.receivepack = 0
I first got convinced by Eric, but I now think you're right.
Eric's point was (I think) not about multiple entries for the same
variable, but about multiple entries for different services, like
http.receivepack = 1
http.uploadpack = 0
The order of assignments to svn-enabled change, but it doesn't matter
because svc is just a local variable pointing to the right element of
rpc_service[i]. So in both cases, you'll assign
rpc_service[index of service].enabled = last occurence of variable
http.service
even though the order of assignments will change.
Eric, am I interpreting right?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git detects changes after new branch created and switched even though files have not been changed
W dniu 2014-08-06 17:04, Jared Brank pisze: Running on Windows with latest version of Git. 1.9.4. Also had this trouble with 1.8.2 but upgraded hoping it would fix the problem. When I do a reset on several files, it says they were successfully reverted they still show up as modified when I do a git status. In this case it's just a newline character at the end of the file. The problem might be the options (gitconfig and gitattributes) affecting how Git deals with end-of-line characters. -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
W dniu 2014-08-01 22:55, Nico Williams pisze: On Fri, Aug 1, 2014 at 3:50 PM, Jonathan Nieder jrnie...@gmail.com wrote: Jonathan Nieder wrote: Do you mean that git merge should be aware of what changes you have already cherry-picked? It isn't, and that's deliberate That said, when today's git merge fails to resolve conflicts, it's easily possible that we could do better at resolving the merge by walking through both sides and understanding what happened. It would help if cherry-pick history where recorded somewhere (beyond the reflog)... Cherry-picks should record two parents, like merges. (Of course, it does no good to know about an unreachable parent, when a commit with two parents is pushed to a repo that doesn't have one of those parents, which can happen when topic branches aren't pushed upstream.) There was (long time ago) a long thread about idea of adding some kind of 'weak' references (links), 'weakparent' that can be automatically used by Git but do not pollute the commit message, and do not affect reachability calculations. Ultimately it went nowhere (as you can see) - there were many problems. For example: how it would work for reverts and rebases? -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
On Wed, Aug 6, 2014 at 10:58 AM, Jakub Narębski jna...@gmail.com wrote: W dniu 2014-08-01 22:55, Nico Williams pisze: It would help if cherry-pick history where recorded somewhere (beyond the reflog)... Cherry-picks should record two parents, like merges. (Of course, it does no good to know about an unreachable parent, when a commit with two parents is pushed to a repo that doesn't have one of those parents, which can happen when topic branches aren't pushed upstream.) There was (long time ago) a long thread about idea of adding some kind of 'weak' references (links), 'weakparent' that can be automatically used by Git but do not pollute the commit message, and do not affect reachability calculations. Ultimately it went nowhere (as you can see) - there were many problems. My proposal was to put this sort of ancillary history info in a branch object (and that branches should be objects). This would have a number of benefits, not the least of which is that at push time you can drop such ancillary history without having to alter the commits being pushed. For example: how it would work for reverts and rebases? Reverts upstream? The revert should record the commit hash of the commit it reverts (but file-level reverts lose), so that this could be noticed. Rebases upstream? Well, that shouldn't happen, but if it does then you must rebase --onto and any cherry-picks of upstream rebased commits lose their ties to those (but this can be detected). In general recording more metadata (assuming there's not privacy issues to consider) can't hurt. Using it might, but having the option to can also help. Nico -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2014, #01; Fri, 1)
Jeff King p...@peff.net writes: On Fri, Aug 01, 2014 at 03:01:31PM -0700, Junio C Hamano wrote: * jk/stash-list-p (2014-07-30) 7 commits - SQUASH??? future-proof, log --cc should imply -p without being told - stash: show combined diff with stash show - stash: default listing to --cc --simplify-combined-diff - add --simplify-combined-diff option - pretty: make empty userformats truly empty - pretty: treat --format= as an empty userformat - revision: drop useless string offset when parsing --pretty Teach git stash list -p to DWIM to git stash list -p --cc, with even nicer twist to collapse combined diff from identical two parents into a regular diff. What do you want to do with this topic? I think we want to drop the stash show patch, based on the discussion we had. The first three patches are nominally prep for that final patch, but actually are things I've often wanted over the years. I'd be glad if they made it in separately, but there were some compatibility questions. I am not sure what compatibility you are worried about. The empty format one looks like a pure bugfix to me, and I agree that they are good changes regardless of the remainder of the series. As clever as I find the --simplify-combined-diff patch, I think we came to the conclusion that --first-parent is probably the reasonable choice. It matches stash show, and it's simple and obvious. Do we just want a patch to specify --first-parent to stash-log? That would make -p just work. The only downside is that there isn't a good way to turn it off. Perhaps we can add --no-first-parent to countermand it? Is it enough to say if you want to do something clever, use git-log? Or do we want to scrap the whole thing and try to update the documentation to make it more clear why -p by itself doesn't do anything? The latter is the most conservative, but it may be too conservative to be useful ;-). Unless we stop advertising that stash list is a thin wrapper around log -g with options that would be useful to view the stash, which is a strange ref with useful reflog entries, the --first-parent approach would be the most sensible, I would say. If we can dissociate stash list from log (in other words, the set of options stash list takes does not have anything to do with the underlying log, even though both may have -p to tell them to give patches, etc.), it would be a totally different matter, and it might give us a better future, but I suspect it might be a bit too late for that. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Subtree with submodule inside?
Is this even possible? The .gitmodule file has to be at the root of the repository, AFAIK. So if the subtree is inherently not at the root, how does it manage its own submodules? Basically I have a common library that also keeps a submodule of third party dependencies (binaries). Each super project that depends on this common library (each super project has its own repository) will add it as a subtree. So what happens to the submodule with this setup? Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] Documentation: git-init: template directory: reword
Linus Arver linusar...@gmail.com writes: No, the unindenting/removal of blank lines is a non-grammar change and is not necessary, as it doesn't have any effect on the actual output (html/txt/manpage). I can either keep the same coding style with the rewording, or chop this into two commits, one for the rewording and another for reformatting. Which one do you suggest? If I were doing this change, I wouldn't touch the formatting, because I did not find that the reformatted version would be any easier to read or maintain compared to the original. But I suspect that you must have thought the reformatting was a good thing to do for a reason, and I suspected I might have been missing something obvious to you, and that was why I asked. If there is a good reason to reformat, then lets hear it in the commit log message of one of the two patches. Otherwise we can drop the reformatting part. So my suggestion would be a patch #1 to reword, and optionally another patch #2 to reformat on top, if (and only if) there is a good reason to reformat. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug v1.9.3: Spurious whitespace warnings when using git apply with the --exclude option
Bug v1.9.3: Spurious whitespace warnings when using git apply with the --exclude option REPRO STEPS: 1. Create a patch PATCH that modifies two files, A and B. Make sure the modifications to file A add whitespace errors and the modifications to file B do not. 2. Apply that patch to a repo using git apply PATCH --exclude A ACTUAL RESULT: git apply outputs a warning: n lines add whitespace errors.” where n is the number of lines with whitespace errors in A. EXPECTED RESULT: No warning about adding whitespace errors because no whitespace errors are added to the tree since A is excluded and B has no whitespace errors.-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] Documentation: git-init: --separate-git-dir: clarify
Linus Arver linusar...@gmail.com writes: On Tue, Aug 05, 2014 at 03:12:21PM -0700, Junio C Hamano wrote: Linus Arver linusar...@gmail.com writes: Signed-off-by: Linus Arver linusar...@gmail.com --- You would need to work on your justification skills ;-) in the log message. What does this change clarify and in what way? Oops, sorry. I guess I should have written some more information in the commit message, something like this: Use shorter sentences to describe what actually happens. We describe what the term Git symbolic link actually means. Also, we separate out the description of the behavioral change upon reinitialization into its own paragraph. Sounds very sensible. Documentation/git-init.txt | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt index f21b85b..bf0a7ae 100644 --- a/Documentation/git-init.txt +++ b/Documentation/git-init.txt @@ -57,12 +57,12 @@ DIRECTORY section below.) --separate-git-dir=git dir:: -Instead of initializing the repository where it is supposed to be, -place a filesytem-agnostic Git symbolic link there, pointing to the -specified path, and initialize a Git repository at the path. The -result is Git repository can be separated from working tree. If this -is reinitialization, the repository will be moved to the specified -path. +Separate the Git repository from your working tree. Instead of initializing the +repository as a directory to either `$GIT_DIR` or `./.git/`, create a text file +there containing the path to the actual repository. This file acts as +filesystem-agnostic Git symbolic link to the repository. While I agree that it is a very good idea to state what it does, what it is for with the very first sentence of the paragraph, separate the git repository from your working tree does not say much more than the name of the option --separate-git-dir already tells the reader. And I do not offhand think of a better version (and obviously I didn't think of any when the current text was reviewed and committed). The second sentence in your version is definitely an improvement over the first and the second sentences of the original (where it is supposed to be does not give any new information to those who don't know, and does not help those who already know). Perhaps we can simply remove the first sentence from your version? +If this is reinitialization, the repository will be moved to the specified path. --shared[=(false|true|umask|group|all|world|everybody|0xxx)]:: -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/7] Documentation: git-init: flesh out example
Linus Arver linusar...@gmail.com writes: On Tue, Aug 05, 2014 at 03:14:48PM -0700, Junio C Hamano wrote: Linus Arver linusar...@gmail.com writes: Signed-off-by: Linus Arver linusar...@gmail.com --- Documentation/git-init.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt index b94d165..16e9f9c 100644 --- a/Documentation/git-init.txt +++ b/Documentation/git-init.txt @@ -138,10 +138,12 @@ Start a new Git repository for an existing code base:: $ cd /path/to/my/codebase $ git init 1 $ git add . 2 +$ git commit3 I agree it is a good discipline to make the initial pristine import immediately after git add . without doing anything else. Perhaps the description below wants to make it more explicit? I could add a comment like the following: For new repositories, creating a commit immediately after git add . is good practice as it will cleanly separate any preexisting work (done under some other VCS, for example) from any new work done with git. Does this make sense? I am not sure how explicit you want it to be, or whether I captured what you wanted to be explained. I was thinking more along the lines of 3 Record the pristine state as the first commit in the history. which should suffice without becoming excessively verbose. Actually, I would like to know if anything is special about the root-commit... As far as Git is concerned, they are just ordinary commits without any parents. A commit in Git can have zero or more parents, so from that structural point of view, they are not that special. They are considered special by users because they represent the beginning of the project history. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
Robert Dailey rcdailey.li...@gmail.com writes: Is this even possible? The .gitmodule file has to be at the root of the repository, AFAIK. So if the subtree is inherently not at the root, how does it manage its own submodules? Basically I have a common library that also keeps a submodule of third party dependencies (binaries). Each super project that depends on this common library (each super project has its own repository) will add it as a subtree. So what happens to the submodule with this setup? My knee-jerk reaction would be subtree would break submodules badly, don't use it ;-). After all, I invented subtree merge as an ugly interim workaround before submodule subsystem got into a usable shape, hoping that new projects can use submodules without resorting to subtree merges. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Apple violating git LGPL?
Hello, I ran git on my newly-set-up OS X Mavericks machine, and get: $ git Agreeing to the Xcode/iOS license requires admin privileges, please re-run as root via sudo. Running 'git --verision' gives the same result. This seems problematic in a few ways, and I am wondering if the git community is interested in addressing it: 1. Why do I have to agree to Apple's licensing terms to use an LGPL program? Is this appropriate? Is it allowed under the LGPL? 2. This is a significant problem for me, because I'm using a work machine and do not have admin access. 3. The version of git I ran is clearly NOT a plain vanilla official git, it is a derivative work. Has Apple provided the source code of the special version that I just ran? If not, that would seem to be a violation of the LGPL. Thanks, Bob -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
On Wed, Aug 6, 2014 at 12:51 PM, Junio C Hamano gits...@pobox.com wrote: My knee-jerk reaction would be subtree would break submodules badly, don't use it ;-). After all, I invented subtree merge as an ugly interim workaround before submodule subsystem got into a usable shape, hoping that new projects can use submodules without resorting to subtree merges. (Sorry Junio, I forgot to Reply all and accidentally sent this only to you. Resending to group) Hi Junio. I agree it certainly does sound evil and defeats the purpose of the goal of ultimately creating a simple workflow :) Could you go over how you feel the submodule system got into usable shape? It still seems to still have all of the problems I still hate: * If you branch a production branch to implement a feature that also impacts the submodule, you need to branch the submodule. There is no easy interface for this and rather clumbsy, especially if people are using GUI tools like SourceTree (most of the people on my team are). * Pull requests do not include submodule changes in the review (we use Atlassian Stash). So 1 pull request per submodule has to be performed and they have to be merged in the appropriate order. * Submodules complicate an otherwise beautiful workflow that Git is supposed to provide (branching) in many ways, and also there are restrictions on the order in which you modify parent repository vs submodule. If I'm going to put my common code in a separate module for the purposes of splitting apart projects that depend on the common code, I want the workflows that Git provides to remain simple. Subtrees seem to do that, you just sync with the common repo every now and then. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Branch objects (was: Re: cherry picking and merge)
On Wed, Aug 6, 2014 at 6:26 PM, Nico Williams n...@cryptonector.com wrote: On Wed, Aug 6, 2014 at 10:58 AM, Jakub Narębski jna...@gmail.com wrote: W dniu 2014-08-01 22:55, Nico Williams pisze: It would help if cherry-pick history where recorded somewhere (beyond the reflog)... Cherry-picks should record two parents, like merges. (Of course, it does no good to know about an unreachable parent, when a commit with two parents is pushed to a repo that doesn't have one of those parents, which can happen when topic branches aren't pushed upstream.) There was (long time ago) a long thread about idea of adding some kind of 'weak' references (links), 'weakparent' that can be automatically used by Git but do not pollute the commit message, and do not affect reachability calculations. Ultimately it went nowhere (as you can see) - there were many problems. My proposal was to put this sort of ancillary history info in a branch object (and that branches should be objects). This would have a number of benefits, not the least of which is that at push time you can drop such ancillary history without having to alter the commits being pushed. Is it something like object-ified reflog, similar to how replacement objects (git-replace) can be thought to be object-ified grafts (I know they are more)? Do I understand it correctly? Did you plan to (ab)use known object types: tree and commit (a bit similar to git-replace and git-note object, though there is no need for fanout trees - the top level tree can reproduce refs hierarchy)? I see that you planned to (ab)use existing transfer mechanism of refs and objects... BTW. sometimes I do wonder if we are not making a mistake trying to shoehorn new features like stash, replacements and notes into DAG, objects (commit, tree, blob), refs and reflogs. I'd rather Git did not make the same mistake (well, I think it was a mistake) that Mercurial did with .hgtags file, (ab)using file transfer for tags, instead of adding separate transfer mechanism like Git has... which led to contortions in interpreting / deling with said file (most recent version is used, not the one in checked out revision) and things like having to commit creating a tag for it to be transferrable. For example: how it would work for reverts and rebases? Reverts upstream? The revert should record the commit hash of the commit it reverts (but file-level reverts lose), so that this could be noticed. If it is object-ified reflog then reverts are not a problem... Rebases upstream? Well, that shouldn't happen, but if it does then you must rebase --onto and any cherry-picks of upstream rebased commits lose their ties to those (but this can be detected). With rebases the problem is that it would be nice to have (at least for a short time) the history of series of patches (the metahistory, or history of a branch), but usually one doesn't need old pre-rebase version after cleaning up the history for publishing. In general recording more metadata (assuming there's not privacy issues to consider) can't hurt. Using it might, but having the option to can also help. True... -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apple violating git LGPL?
On Wed, Aug 06, 2014 at 02:10:08PM -0400, Robert P Fischer wrote: 3. The version of git I ran is clearly NOT a plain vanilla official git, it is a derivative work. Has Apple provided the source code of the special version that I just ran? If not, that would seem to be a violation of the LGPL. I found the source code of Apple's Git builds here: http://www.opensource.apple.com/source/Git/ Mine happens to be Git-48. I'm not sure how to tell what version you have if it is prompting you to accept a licence first. Charles. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
On Aug 6, 2014, at 8:43 AM, Jakub Narębski jna...@gmail.com wrote: I gave a solution for git using branches and it works just fine. It retains the simple 3-point merge as well. It works for this simple case, but I think it has unfortunate potential to go silently wrong. That just means that you want to have two commands. One for the people that when the remove a patch, they want it gone. The other for people that when they remove a patch, they want it to magically reappear. I’m of the former class of individuals. Now, I would argue that that is the wrong solution of course. See below for uncherry-pick. Now, if I needed a solution to the one problem that was mentioned, I would then request an uncherry-pick command to undo the cherry-pick. The semantics of it are, the patch is removed from the tree, and when merged, that patch isn’t removed from the source. See, we then retain the useful property that everything that should work does, and the system is predictable because it then does exactly what the user said to do. Conceptually of course, it doesn’t have anything to do with cherry, if you merge a branch accidentally, and then remove it, and merge it, I think you still wind up with the work being removed. Conceptually, it is just an undo a change, cherry, merge, file rename, whatever. Now, why is this preferable? Because the advanced user gets to explain what they want to git, and then git does what they want. It also works for beginning users, it does what they ask it to do. If you are afraid you know better what command that they really wanted to use instead of the command they are using, you can prompt them and ask, did you mean this or that? After 20 times being asked, it would get old and then even a new user would just issue the commands they want. I’m not in favor of that, I’d prefer that the system just do what they tell it to do. Also, it prevents fully removing (commits, not only refs) the branch you cherry-picked from. The commit you cherry picked may no longer be (or may no longer should be) in the repository. I’m picking from trunk, when it goes, I go. :-) Also, this could be avoided by using feature branches and merging instead of committing to one branch and cherry-picking to other branches. If the problem remains unfixed, at least the documentation should be changed to say cherry will mess up merge. If you never merge, never a problem. For me, I would read that, and say, well, trivially, cherry isn’t for me (til they fix the bug that causes it to mess up merges). I can’t see anything on http://git-scm.com/docs/git-cherry-pick which says it will mess up merges.-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] stash: default listing to working-tree diff
On Wed, Aug 06, 2014 at 10:12:25AM -0700, Junio C Hamano wrote:
I think we want to drop the stash show patch, based on the discussion
we had. The first three patches are nominally prep for that final
patch, but actually are things I've often wanted over the years. I'd be
glad if they made it in separately, but there were some compatibility
questions.
I am not sure what compatibility you are worried about. The empty
format one looks like a pure bugfix to me, and I agree that they
are good changes regardless of the remainder of the series.
I was mostly worried that somebody is relying on the weird current
behavior with the blank line. I'm inclined to call it a bugfix, too.
As clever as I find the --simplify-combined-diff patch, I think we came
to the conclusion that --first-parent is probably the reasonable
choice. It matches stash show, and it's simple and obvious. Do we just
want a patch to specify --first-parent to stash-log? That would make
-p just work. The only downside is that there isn't a good way to turn
it off.
Perhaps we can add --no-first-parent to countermand it?
I started down that road and then realized that --first-parent is not
enough. It is only interesting combined with -m. But it turns out that
using the two together does exactly what we want, and is overridden
as you would hope with just --cc.
See the patch below, which I think could replace the top three from
jk/stash-list-p (or really, could replace the whole series, and the
bottom three could go into their own topic).
-- 8 --
Subject: stash: default listing to working-tree diff
When you list stashes, you can provide arbitrary git-log
options to change the display. However, adding just -p
does nothing, because each stash is actually a merge commit.
This implementation detail is easy to forget, leading to
confused users who think -p is not working. We can make
this easier by defaulting to --first-parent -m, which will
show the diff against the working tree. This omits the
index portion of the stash entirely, but it's simple and it
matches what git stash show provides.
People who are more clueful about stash's true form can use
--cc to override the -m, and the --first-parent will
then do nothing. For diffs, it only affects non-combined
diffs, so --cc overrides it. And for the traversal, we are
walking the linear reflog anyway, so we do not even care
about the parents.
Signed-off-by: Jeff King p...@peff.net
---
git-stash.sh | 2 +-
t/t3903-stash.sh | 42 ++
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index bcc757b..9c1ba8e 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -297,7 +297,7 @@ have_stash () {
list_stash () {
have_stash || return 0
- git log --format=%gd: %gs -g $@ $ref_stash --
+ git log --format=%gd: %gs -g --first-parent -m $@ $ref_stash --
}
show_stash () {
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 5b79b21..1e29962 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -685,4 +685,46 @@ test_expect_success 'handle stash specification with
spaces' '
grep pig file
'
+test_expect_success 'setup stash with index and worktree changes' '
+ git stash clear
+ git reset --hard
+ echo index file
+ git add file
+ echo working file
+ git stash
+'
+
+test_expect_success 'stash list implies --first-parent -m' '
+ cat expect -\EOF
+ stash@{0}: WIP on master: b27a2bc subdir
+
+ diff --git a/file b/file
+ index 257cc56..d26b33d 100644
+ --- a/file
+ +++ b/file
+ @@ -1 +1 @@
+ -foo
+ +working
+ EOF
+ git stash list -p actual
+ test_cmp expect actual
+'
+
+test_expect_success 'stash list --cc shows combined diff' '
+ cat expect -\EOF
+ stash@{0}: WIP on master: b27a2bc subdir
+
+ diff --cc file
+ index 257cc56,9015a7a..d26b33d
+ --- a/file
+ +++ b/file
+ @@@ -1,1 -1,1 +1,1 @@@
+ - foo
+-index
+ ++working
+ EOF
+ git stash list -p --cc actual
+ test_cmp expect actual
+'
+
test_done
--
2.1.0.rc0.286.g5c67d74
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apple violating git LGPL?
On Wed, Aug 06, 2014 at 02:10:08PM -0400, Robert P Fischer wrote: I ran git on my newly-set-up OS X Mavericks machine, and get: $ git Agreeing to the Xcode/iOS license requires admin privileges, please re-run as root via sudo. Running 'git --verision' gives the same result. This seems problematic in a few ways, and I am wondering if the git community is interested in addressing it: 1. Why do I have to agree to Apple's licensing terms to use an LGPL program? Is this appropriate? Is it allowed under the LGPL? You are not running the real git at all here; Apple ships stubs for several commands, including git, that are distributed with XCode. When you run the program, it prompts you to install XCode, which then actually downloads and installs git, replacing the stub. You can download XCode separately, of course, but you would still need to agree to the license. You can download git separately and not need to bother (though I do not know offhand how to download Apple's git, or whether they provide a script to replace the stubs). Also, minor nit, but git is GPL, not LGPL. 2. This is a significant problem for me, because I'm using a work machine and do not have admin access. You can always download and install git yourself. There's no need to use the XCode version. However, XCode is the simplest way to get a compiler on the machine, AFAIK (I do not use OS X myself). 3. The version of git I ran is clearly NOT a plain vanilla official git, it is a derivative work. Has Apple provided the source code of the special version that I just ran? If not, that would seem to be a violation of the LGPL. You didn't run a derivative git. You ran their stub. As it happens, though, they _do_ modify the git that they distribute. I know at least that they bake-in the osxkeychain helper config in away that the user cannot turn off. There may be more changes, but I haven't done a full diff. And they do provide the source: https://www.opensource.apple.com/source/Git/ -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-gui: make gc warning threshold match 'git gc --auto'
The number of loose objects at which git-gui shows a gc warning has
historically been hardcoded to ~2000, or ~200 on Windows. The warning can
only be disabled completely via gui.gcwarning=false.
Especially on Windows, the hardcoded threshold is so ridiculously low that
git-gui often complains even immediately after gc (due to loose objects
only referenced by the reflog).
'git gc --auto' uses a much bigger threshold to check if gc is necessary.
Additionally, the value can be configured via gc.auto (default 6700).
There's no special case for Windows.
Change git-gui so that it only warns if 'git gc --auto' would also do an
automatic gc, i.e.:
- calculate the threshold from the gc.auto setting (default 6700,
disabled if = 0)
- check directory .git/objects/17
We still check four directories (14-17) if gc.auto is very small, to get a
better estimate.
Signed-off-by: Karsten Blees bl...@dcon.de
---
git-gui/lib/database.tcl | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/git-gui/lib/database.tcl b/git-gui/lib/database.tcl
index 1f187ed..212b195 100644
--- a/git-gui/lib/database.tcl
+++ b/git-gui/lib/database.tcl
@@ -89,19 +89,26 @@ proc do_fsck_objects {} {
}
proc hint_gc {} {
+ global repo_config
+ set auto_gc $repo_config(gc.auto)
+ if {$auto_gc eq {}} {
+ set auto_gc 6700
+ } elseif {$auto_gc = 0} {
+ return
+ }
+
set ndirs 1
- set limit 8
- if {[is_Windows]} {
+ set limit [expr {($auto_gc + 255) / 256}]
+ if {$limit 4} {
set ndirs 4
- set limit 1
}
set count [llength [glob \
-nocomplain \
-- \
- [gitdir objects 4\[0-[expr {$ndirs-1}]\]/*]]]
+ [gitdir objects 1\[[expr {8-$ndirs}]-7\]/*]]]
- if {$count = $limit * $ndirs} {
+ if {$count $limit * $ndirs} {
set objects_current [expr {$count * 256/$ndirs}]
if {[ask_popup \
[mc This repository currently has approximately %i
loose objects.
--
2.0.3.921.ga7e731a.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-bitmap: do not use gcc packed attribute
Am 05.08.2014 20:47, schrieb Jeff King: On Mon, Aug 04, 2014 at 09:19:46PM +0200, Karsten Blees wrote: Hmm, I wonder if it wouldn't be simpler to read / write the desired on-disk structure directly, without copying to a uchar[6] first. Probably. My initial attempt was to keep together the read/write logic about which sizes each item is, but I think the result ended up unnecessarily verbose and harder to follow. Yeah, having read / write logic in different files is confusing, esp. when not using structs to define the file format. Here's what I came up with (just a sketch, commit message is lacky and the helper functions deserve a better place / name): I like it. Want to clean it up and submit in place of mine? Will do, but it will have to wait till next week. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
On Aug 1, 2014, at 4:40 PM, Nico Williams n...@cryptonector.com wrote: As for rebase, I still don't understand why it doesn't work for you. http://git-scm.com/docs/git-rebase says: Rebasing (or any other form of rewriting) a branch that others have based work on is a bad idea If you read stack-overflow, you will discover a ton of people that like doing this, and they get hammered because of it. My use case fits exactly (as near as I can tell) the hammer case. If you make a different command that isn’t guaranteed to screw me and my co-workers over, and tell us to use that, then I’d be happy to use it. Bet the farm that it won’t bite you, just to be bitten isn’t what I want to try and recover from. You didn't really explain. If you say it will never bit you and then fix all the documentation to not say it will bite you… I’d be happy to contemplate it again. Now, I found the stack-overflow commentary first, and all the horrors of it, and all the nuances. I carefully read what people were doing, how what I wanted to related to what they were doing, and it sure felt like I was in the, don’t go there camp. Suppose we're right and it's the right solution for you, then you might be ecstatic, but you gotta try it first. So, I like to know if I’m driving off a cliff, before I do. I’m the type of person that would rather know were the road goes, and merely avoid driving off the cliff. When stack-overflow is littered with the bodies of people that thought it would be fun, I tend to just say, that’s not for me. The only case where I can imagine not using a rebase-heavy workflow is where I have to track multiple forked upstreams and so I want to merge each into my branch. So, sounds like I fit that use case and rebase could be my friend. How do I square what you said and: Rebasing (or any other form of rewriting) a branch that others have based work on is a bad idea ? I want all old refs in old emails to work. I want all refs in bugzilla to work. I want to see the original dates of all the work. I want git blame to report those artifacts in email and bugzilla. I have coworkers that I push to, pull from (through a single sharing point, we call the master tree). We work on gcc, we pull git gcc down to a local copy, then merge it into our tree. I want to cherry pick changes from upstream. I do work and push to our master, I pull work of coworkers from the master, my coworkers do the same. Isn’t this the canonical open source use case? (I find that many users are allergic to rebasing. Many people have told me that rebase is lying, that history must be immutable, and so on, all ignoring that: git users don't rebase published branches, So, when I push, and someone else pulls, is that published? I thought it was.-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-bitmap: do not use gcc packed attribute
Karsten Blees karsten.bl...@gmail.com writes: Am 05.08.2014 20:47, schrieb Jeff King: On Mon, Aug 04, 2014 at 09:19:46PM +0200, Karsten Blees wrote: Hmm, I wonder if it wouldn't be simpler to read / write the desired on-disk structure directly, without copying to a uchar[6] first. Probably. My initial attempt was to keep together the read/write logic about which sizes each item is, but I think the result ended up unnecessarily verbose and harder to follow. Yeah, having read / write logic in different files is confusing, esp. when not using structs to define the file format. Here's what I came up with (just a sketch, commit message is lacky and the helper functions deserve a better place / name): I like it. Want to clean it up and submit in place of mine? Will do, but it will have to wait till next week. Thanks, both. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rebase safely (Re: cherry picking and merge)
On Wed, Aug 06, 2014 at 12:11:16PM -0700, Mike Stump wrote: On Aug 1, 2014, at 4:40 PM, Nico Williams n...@cryptonector.com wrote: As for rebase, I still don't understand why it doesn't work for you. http://git-scm.com/docs/git-rebase says: Rebasing (or any other form of rewriting) a branch that others have based work on is a bad idea If you read stack-overflow, you will discover a ton of people that like doing this, and they get hammered because of it. My use case fits exactly (as near as I can tell) the hammer case. It's not a good idea to rebase a branch in a repo that others pull from. There's nothing wrong with rebasing your patches on that same branch in your clone as long as the end result is a fast forward merge when you push. $ git clone https:///blah $ cd blah $ do some work $ git commit ... $ git fetch origin ---$ git rebase origin/master $ git push origin master There's NOTHING wrong with that rebase. It's perfectly safe because it's only in your *private* clone. (If later you publish that clone and expect people to track your master branch, then rebase becomes problematic, but that's not something most people ever do with their clones.) The only use-case I've seen where a rebase-based workflow doesn't work is where you have multiple upstreams that you're following. I.e., the upstream forked and you want to take some commits from one, some from the other, or otherwise keep a merge of both. (Also, if an upstream is ever rebased you can usually recover on the downstream side by rebasing with the --onto option, so it's not the end of the world.) Now, I found the stack-overflow commentary first, and all the horrors of it, and all the nuances. I carefully read what people were doing, how what I wanted to related to what they were doing, and it sure felt like I was in the, don’t go there camp. A lot of people rant about rebase. They're wrong. They've distorted your perspective. So, I like to know if I’m driving off a cliff, before I do. I’m the [...] There's just two simple rules to follow and you'll be safe: 1) NEVER git push -f (--force) to a published repo/branch. The upstream should enforce this with a receive hook. 2) NEVER work directly in a published repo. Instead work in a private clone. To help make sure of this, never publish a non-bare repo (bare == has no workspace; non-bare == has a workspace). If you ever do a rebase that produces results you're unhappy with you can undo that rebase like so: - use git reflog to find the branch's previous HEAD commit - reset the branch to point to that commit It really helps to think of git as a pile of commits arranged in a Merkle has tree. Branches and tags are just symbolic names for specific commits. Rebase builds a new line of commits in the tree then it changes the symbolic branch name's HEAD to point to the head of that new line of commits, BUT NOTHING IS LOST in the pile of commits that is the repo, not until you git-prune(1) to remove commits not reachable from symbolic names (branches and tags). The only case where I can imagine not using a rebase-heavy workflow is where I have to track multiple forked upstreams and so I want to merge each into my branch. So, sounds like I fit that use case and rebase could be my friend. Excellent. How do I square what you said and: Rebasing (or any other form of rewriting) a branch that others have based work on is a bad idea ? See above. I want all old refs in old emails to work. I want all refs in They will if you stick to the two rules I mention above. bugzilla to work. I want to see the original dates of all the work. Ditto. I want git blame to report those artifacts in email and bugzilla. I have coworkers that I push to, pull from (through a single sharing point, we call the master tree). We work on gcc, we pull git gcc down to a local copy, then merge it into our tree. I want to cherry pick changes from upstream. I do work and push to our master, I pull work of coworkers from the master, my coworkers do the same. Isn’t this the canonical open source use case? That means that you have/maintain an intermediate upstream, yes? This is a bit trickier since once in a while that intermediate upstream and everyone downstream of it has to catch up with the real upstream. Here you have two options: - the intermediate diverges from the real upstream, and then you merge/cherry-pick from the upstream as needed The intermediate's maintainer must still merge/rebase/cherry-pick from the intermediate branch and onto a branch of the upstream in order to push to the upstream. or - the intermediate occasionally rebases onto the upstream, and then the repos downstream of the intermediate must also rebase with --onto. In this case the intermediate's maintainer must tell the downstreams what rebase command to execute. This makes it easier to push from
Re: [PATCH 02/11] http-backend.c: replace `git_config()` with `git_config_get_bool()` family
On Wed, Aug 6, 2014 at 11:44 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Tanay Abhra tanay...@gmail.com writes:
On 8/5/2014 12:29 AM, Eric Sunshine wrote:
On Mon, Aug 4, 2014 at 2:33 PM, Tanay Abhra tanay...@gmail.com wrote:
- if (skip_prefix(var, http., p)) {
- int i;
-
- for (i = 0; i ARRAY_SIZE(rpc_service); i++) {
- struct rpc_service *svc = rpc_service[i];
- if (!strcmp(p, svc-config_name)) {
- svc-enabled = git_config_bool(var, value);
- return 0;
- }
- }
+ for (i = 0; i ARRAY_SIZE(rpc_service); i++) {
+ struct rpc_service *svc = rpc_service[i];
+ strbuf_addf(var, http.%s, svc-config_name);
+ if (!git_config_get_bool(var.buf, value))
+ svc-enabled = value;
+ strbuf_reset(var);
}
There is a behavior change here. The original code set svc-enabled to
the first matching rpc_service[] entry, whereas the new code sets it
to the last matching entry. Is this change intentional?
I was preparing the reroll and I saw that I had missed your mail.
I think that I haven't changed the behaviour, the original one is
written in callback form so it has to go through the array every time for
each
new value.
When there are multiple entries for a service say,
http.receivepack = 1
http.receivepack = 0
I first got convinced by Eric, but I now think you're right.
Eric's point was (I think) not about multiple entries for the same
variable, but about multiple entries for different services, like
http.receivepack = 1
http.uploadpack = 0
The order of assignments to svn-enabled change, but it doesn't matter
because svc is just a local variable pointing to the right element of
rpc_service[i]. So in both cases, you'll assign
rpc_service[index of service].enabled = last occurence of variable
http.service
even though the order of assignments will change.
Eric, am I interpreting right?
Yes. During initial review, I either didn't read the old code closely
enough or I misinterpreted it. Upon re-read, Tanay's rewrite looks
fine.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Branch objects (was: Re: cherry picking and merge)
On Wed, Aug 06, 2014 at 08:31:18PM +0200, Jakub Narębski wrote: On Wed, Aug 6, 2014 at 6:26 PM, Nico Williams n...@cryptonector.com wrote: My proposal was to put this sort of ancillary history info in a branch object (and that branches should be objects). This would have a number of benefits, not the least of which is that at push time you can drop such ancillary history without having to alter the commits being pushed. Is it something like object-ified reflog, similar to how replacement objects (git-replace) can be thought to be object-ified grafts (I know they are more)? Do I understand it correctly? Yes, per-branch. At push time a commit would be pushed to the upstream branch listing the commits pushed now (and who by). Locally every rebase/cherry-pick/merge/commit onto the branch would appear in the branch object's history, kinda just like the reflog. The main difference is that the upstream branch's history could be viewed. Did you plan to (ab)use known object types: tree and commit (a bit similar to git-replace and git-note object, though there is no need for fanout trees - the top level tree can reproduce refs hierarchy)? I see that you planned to (ab)use existing transfer mechanism of refs and objects... Just like signed tags, basically. Reverts upstream? The revert should record the commit hash of the commit it reverts (but file-level reverts lose), so that this could be noticed. If it is object-ified reflog then reverts are not a problem... Right. Rebases upstream? Well, that shouldn't happen, but if it does then you must rebase --onto and any cherry-picks of upstream rebased commits lose their ties to those (but this can be detected). With rebases the problem is that it would be nice to have (at least for a short time) the history of series of patches (the metahistory, or history of a branch), but usually one doesn't need old pre-rebase version after cleaning up the history for publishing. Right. In general recording more metadata (assuming there's not privacy issues to consider) can't hurt. Using it might, but having the option to can also help. True... The principle should be to record as much metadata as possible, pruning ancillary metadata (reflog-like metadata that isn't on the commits) only at push time. Nico -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug v1.9.3: Spurious whitespace warnings when using git apply with the --exclude option
Peyton Randolph prando...@apple.com writes: Bug v1.9.3: Spurious whitespace warnings when using git apply with the --exclude option ... EXPECTED RESULT: No warning about adding whitespace errors because no whitespace errors are added to the tree since A is excluded and B has no whitespace errors. By reading the way the code is structured, I would expect (I didn't check) that all versions of apply that supports whitespace error detection with include/exclude option to share this symptom, and this bug is not specific to 1.9.3. In order to decide if a hunk with a new whitespace error should be reported, the program needs to parse the patch input to at least find out what path the part being parsed is to be applied, and the whitespace error detection and reporting is done when the patch input is parsed. After the part that applies to one path is fully parsed, it then checks to see if the path is included/excluded and omits it from the --stat output and also actual application. That part of the code needs to be restructured so that at least error reporting (and preferrably error detection) is not done when the path is known to be excluded. Let me see if this can be easily done. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Rebase safely (Re: cherry picking and merge)
On Wed, Aug 06, 2014 at 02:44:59PM -0500, Nico Williams wrote: That means that you have/maintain an intermediate upstream, yes? This is a bit trickier since once in a while that intermediate upstream and everyone downstream of it has to catch up with the real upstream. Here you have two options: - the intermediate diverges from the real upstream, and then you merge/cherry-pick from the upstream as needed The intermediate's maintainer must still merge/rebase/cherry-pick from the intermediate branch and onto a branch of the upstream in order to push to the upstream. I should add something important here. Rebasing makes life easier for the intermediate maintainer, and for any upstream maintainer who has to merge pull requests or patches sent in email. Rebasing puts the onus for merging on the contributor, exactly where it belongs! (Granted, for an e-mail based workflow one's patches might have made for a fast-forward merge when sent but not when the upstream gets to them. With long enough latency this gets painful. Which is why I don't recommend an e-mail based commit integration workflow.) Nico -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
Am 06.08.2014 um 20:18 schrieb Robert Dailey: On Wed, Aug 6, 2014 at 12:51 PM, Junio C Hamano gits...@pobox.com wrote: My knee-jerk reaction would be subtree would break submodules badly, don't use it ;-). After all, I invented subtree merge as an ugly interim workaround before submodule subsystem got into a usable shape, hoping that new projects can use submodules without resorting to subtree merges. (Sorry Junio, I forgot to Reply all and accidentally sent this only to you. Resending to group) Hi Junio. I agree it certainly does sound evil and defeats the purpose of the goal of ultimately creating a simple workflow :) Could you go over how you feel the submodule system got into usable shape? It still seems to still have all of the problems I still hate: * If you branch a production branch to implement a feature that also impacts the submodule, you need to branch the submodule. There is no easy interface for this and rather clumbsy, especially if people are using GUI tools like SourceTree (most of the people on my team are). There were thoughts about having git branch optionally create a branch in the submodule too. But in a lot of real world scenarios that won't help because the same branch name won't necessarily make sense in superproject and submodule at the same time, so you'd have to create different branches anyway. But if it makes a significant amount of users happy, I think adding --recurse-submodules to git branch might make sense to make life easier for them. I do not know SourceTree myself, but do you have any ideas you'd like to share how you could envision to make branching less tedious? I hope it at least shows you that there are modifications inside the submodule while examining the superproject and provides an easy way to start a new instance inside the submodule to commit the changes there? Otherwise it'd make life unnecessarily hard for you. * Pull requests do not include submodule changes in the review (we use Atlassian Stash). So 1 pull request per submodule has to be performed and they have to be merged in the appropriate order. Which seems to be a chore in your scenario, but is just the Right Thing to do when someone else maintains the submodule. Given the danger of a rebase in the submodule pulling away the commit recorded in the superproject it is considered best practice to always merge in the submodule into a never-to-be-rebased branch first before committing the superproject. But I agree it is twice the work. * Submodules complicate an otherwise beautiful workflow that Git is supposed to provide (branching) in many ways, and also there are restrictions on the order in which you modify parent repository vs submodule. I cannot remember anyone coming up with a better workflow when having distinct repos (superproject and submodule). Subtree on the other hand has other issues (like you can easily forget to push a fix to a subtree to its upstream, which is something submodules make you aware of). And the submodule workflow is as beautiful as ever if you consider superproject and submodule different repos (which they are), and then it is obvious that you have to do everything twice ;-) But I do have the impression that a lot of third party tools are still rather submodule ignorant and of not much help there. If I'm going to put my common code in a separate module for the purposes of splitting apart projects that depend on the common code, I want the workflows that Git provides to remain simple. Subtrees seem to do that, you just sync with the common repo every now and then. If you are only interested in single from upstream into my repo workflow and don't care much about pushing your local changes back to the common code repo, subtrees are easier to use than submodules. But if you need or want to contribute your changes back, I believe submodules gain the upper hand rather quickly. But back to your original question: I do no see a way to import another repo using a submodule via subtree either. You'd have to consistently use submodules to make that work (even if that means that you'll have three repos to think about, one being embedded in the other). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apple violating git LGPL?
On 08/06/2014 11:43 AM, Jeff King wrote: snippage here 8 8 As it happens, though, they _do_ modify the git that they distribute. I know at least that they bake-in the osxkeychain helper config in away that the user cannot turn off. There may be more changes, but I haven't done a full diff. And they do provide the source: https://www.opensource.apple.com/source/Git/ Is that a plugin? if not what about proprietary plugins? How are they affected by the license is this case? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apple violating git LGPL?
Also, minor nit, but git is GPL, not LGPL. But Apple put a LGPL license in side the folder. See: https://www.opensource.apple.com/source/Git/Git-48/src/git/LGPL-2.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apple violating git LGPL?
On Wed, Aug 06, 2014 at 01:23:00PM -0700, Tony wrote: Also, minor nit, but git is GPL, not LGPL. But Apple put a LGPL license in side the folder. See: https://www.opensource.apple.com/source/Git/Git-48/src/git/LGPL-2.1 Interesting. It starts with: While most of this project is under the GPL (see COPYING), the xdiff/ library and some libc code from compat/ are licensed under the GNU LGPL, version 2.1 or (at your option) any later version and some other files are under other licenses. Check the individual files to be sure. which makes sense. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Apple violating git LGPL?
On Wed, Aug 06, 2014 at 12:53:56PM -0700, Bruce Ferrell wrote: On 08/06/2014 11:43 AM, Jeff King wrote: snippage here 8 8 As it happens, though, they _do_ modify the git that they distribute. I know at least that they bake-in the osxkeychain helper config in away that the user cannot turn off. There may be more changes, but I haven't done a full diff. And they do provide the source: https://www.opensource.apple.com/source/Git/ Is that a plugin? if not what about proprietary plugins? How are they affected by the license is this case? I don't know exactly what you mean by plugin here. osxkeychain is a separate program found in git's contrib directory. You point git at it by setting your credential.helper config to osxkeychain. However, in the Apple version, they have hardcoded that config into the git binary, and you can't turn it off (you can add additional helpers, but you can't undo the keychain helper). So I don't think there is any licensing question about what they have done[1]. I do not know offhand of any proprietary credential helpers. They do interact with git over a pipe, and their primary function is to feed data to git. My understanding is that there are some people who believe that makes them derivative works of git (i.e., that talking RPC over a pipe to avoid linking does not get around the GPL), but there are others who would consider them separate programs. -Peff [1] Whether what they have done is smart is another matter. I looked at the diff Apple's Git-48 and v1.8.5.2 (on which it seems to be based). There aren't a huge number of changes, but some of them baffle me. Why bake-in credential.helper when you can set it in /etc/gitconfig? Why default core.trustctime to false when you can set it via /etc/gitconfig? Etc. I wish they would work with the configurability tools that we already provide, and if those are not sufficient, work with us to make git more configurable. But AFAIK whoever is responsible for those changes has never participated on the mailing list. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 8/8] add tests for `git_config_get_string_const()`
Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: ... +grep line 7.*.git/config\|.git/config.*line 7 result +' This is still dependant on the locale (line is translated). You need to use test_i18ngrep instead of grep here (see its definition and comment in t/test-lib.sh). I don't think you need these two alternatives OTOH. BTW, Junio, I don't understand your remark This test is too tight (the full string) in the previous iteration. Can you elaborate? The original test was trying to match the string fully, i.e. + grep fatal: bad config variable '\''alias.br'\'' at file line 2 in .git/config result As I already was feeling funny about seeing the phrase file line in the message and expecting it to be corrected, I thought I should encourage a check that does not depend on minor phrasing changes, if it can be done without bending backwards. I do agree with you that using \| in grep a pattern to trigger ERE Alternation counts as bending backwards as that is a GNU extension and not portable. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 03/19] rebase -i: reword executes pre-commit hook on interim commit
On Mon, Aug 04, 2014 at 08:51:42PM +0200, Fabian Ruch wrote: though I would also not be opposed to some more uniform hook selection mechanism (e.g., --no-verify=pre-commit or something). While the --no-verify= mechanism doesn't add a new option to the git-commit interface but lets one refine the --no-verify option, users might find it weird to have an argument to name disabled hooks but then not be able to disable all hooks that way. Right, I think that is only worth doing if we add it uniformly for all hooks. I wonder if it should be a git option, not one to specific commands. Like: git --disable-hook=pre-commit commit ... and then the run_hook code can just respect the disabled list. I think a name like --disable-hook or your --bypass-hook is better than --no-verify here, too. Not all hooks are about verifying (I also think disable is better than bypass for that reason, too). Since the hook selection wouldn't have to change, the options parsing code seems to be simpler (--bypass-hook= would have to support several occurrences with different arguments which could be implemented as an OPT_CALLBACK?) and git-commit decided to have --no- options for hook selection so far, I would lean towards your patch from above. You'd probably implement --disable-hook with OPT_STRING_LIST (or just do it by hand if it's the git.c option parser). And then the --no-post-rewrite arguments remain for historical compatibility, and can be implemented as synonyms for --disable-hook=post-rewrite, etc. I think people have also asked for the ability to override hooks, too, though I do not remember the exact details. Instead of --disable-hook, we could have an option for setting specific hooks (and setting them to nothing to disable would just be one possibility). This is getting bigger in scope, though. I was trying not to derail you too much from your GSoC project, but see if we could just fix this one hacky corner easily. Since all of the hook options are motivated by internal usage from git-rebase, perhaps they should be configured as PARSE_OPT_HIDDEN. Any thoughts on this? I could go either way. Just because they are motivated by git-rebase does not mean other callers might not find them useful (after all, git commands are often meant to be scripted). As long as we promise to support them in future versions as we do with normal options, I do not think there is any problem in advertising them. That being said, git commit -h is already getting pretty long. It might be worth cutting some seldom-used options from that list just to make it more palatable to normal users. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 07/19] rebase -i: log the replay of root commits
On Mon, Aug 04, 2014 at 11:21:41PM +0200, Fabian Ruch wrote: Thanks for laying out the differences in the user visible output. With stock git we are seeing summaries for other commits, but not root commits, _with the --verbose option_. It's the fault of my patch to show the summary even in non-verbose mode. This is now fixed by wrapping the relevant command in 'output', a shell function defined in git-rebase.sh as follows: Ah, OK. That makes a lot more sense, then. output=$($@ 21 ) status=$? test $status != 0 printf %s\n $output return $status The problem that git-rebase--interactive has is that the redirection of stdin to a variable (or a file) does not work reliably with commands that invoke the log message editor, that is 'reword' and 'squash' produce their output both in verbose and non-verbose mode. I wouldn't know how to fix this but 1) invoking $GIT_EDITOR from git-rebase--interactive.sh, but to make this right there should be a built-in command for shell scripts and an interface for git-commit that prepare the editor contents like git-commit does now, or 2) forcing $GIT_EDITOR and git-commit to print on distinct file descriptors, which would involve temporarily wrapping the $GIT_EDITOR call in a shell script that redirects stdin to some other file descriptor similar to what t/test-lib.sh does, or Hmm. In the test scripts, we send stdout and stderr for sub-commands to fds 3 and 4 respectively. And then we either point those at /dev/null or to 1 and 2, depending on whether $verbose mode was specified. I don't think that will work here, though. You literally have one git commit command to run, and you want its stderr/stdout to go somewhere different than the $GIT_EDITOR it will invoke. Your (2) makes some sense to me. Something like: GIT_EDITOR=$(shellquote $(git var GIT_EDITOR)) 3 24 \ git commit ... 31 42 wherever 21 Or we could just point it at /dev/tty, though I guess that may open another can of worms (systems without /dev/tty, what happens when you do not have a terminal, etc). 3) passing the --quiet option in non-verbose mode and omitting it in verbose mode, which would cover the '$status != 0' above for if a command fails, it should indicate its error status despite being asked to be silent. Yeah, that is probably the cleanest option if it works. I would just worry that it is not as complete. It works for git commit, but are there are other commands wrapped in the verbose output that would want the same treatment (that might not know about --quiet)? Your paragraph below says it would not be that big a deal, so as long as we don't plan to add anything in the future that could not handle the requirement, that may be enough. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
Jens Lehmann wrote: There were thoughts about having git branch optionally create a branch in the submodule too. But in a lot of real world scenarios that won't help because the same branch name won't necessarily make sense in superproject and submodule at the same time So, here is how I think git could behave to make that into a non-issue: 1. Commands like 'git checkout' able to recurse into submodules, so when you switch branches in the superproject, the files in the submodule are at the right commit, too. Luckily your series does that. Ronnie was helping me with thoughts about how to simplify the patch a little, and I'd be happy to talk with or coordinate with anyone else interested (by email or on IRC --- I am jrnieder on Freenode). 2. Submodules aware of their superproject and of the parent's branches. In other words, submodules would act as thought under refs/ they had a symlink parent - ../../../refs So you could do git checkout --recurse-submodules master cd path/to/submodule git checkout parent/heads/next This would avoid danger from git gc in submodules and would get rid of most of the motivation for named branches in the submodule, I'd think. 3. That's it. Sensible? Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Two fixes to git apply
This started as an attempt to fix a long-time bug, in which a partial git apply with --exclude/--include still complained about whitespace breakage in a part that was excluded from patch application. The final patch fixes it. Restructuring of the code necessary to fix it revealing another long-standing bug that is not related X-, which turned out to be a larger fix (patch 1). Junio C Hamano (3): apply: use the right attribute for paths in non-Git patches apply: hoist use_patch() helper for path exclusion up apply: omit ws check for excluded paths builtin/apply.c | 131 --- t/t4119-apply-config.sh | 17 ++ t/t4124-apply-ws-rule.sh | 11 3 files changed, 96 insertions(+), 63 deletions(-) -- 2.1.0-rc1-209-g4e1b551 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] apply: hoist use_patch() helper for path exclusion up
We will be adding a caller to the function a bit earlier in this
file in a later patch.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 81 ++---
1 file changed, 43 insertions(+), 38 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 4270cde..bf075cc 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1938,6 +1938,49 @@ static void prefix_patch(struct patch *p)
}
/*
+ * include/exclude
+ */
+
+static struct string_list limit_by_name;
+static int has_include;
+static void add_name_limit(const char *name, int exclude)
+{
+ struct string_list_item *it;
+
+ it = string_list_append(limit_by_name, name);
+ it-util = exclude ? NULL : (void *) 1;
+}
+
+static int use_patch(struct patch *p)
+{
+ const char *pathname = p-new_name ? p-new_name : p-old_name;
+ int i;
+
+ /* Paths outside are not touched regardless of --include */
+ if (0 prefix_length) {
+ int pathlen = strlen(pathname);
+ if (pathlen = prefix_length ||
+ memcmp(prefix, pathname, prefix_length))
+ return 0;
+ }
+
+ /* See if it matches any of exclude/include rule */
+ for (i = 0; i limit_by_name.nr; i++) {
+ struct string_list_item *it = limit_by_name.items[i];
+ if (!fnmatch(it-string, pathname, 0))
+ return (it-util != NULL);
+ }
+
+ /*
+* If we had any include, a path that does not match any rule is
+* not used. Otherwise, we saw bunch of exclude rules (or none)
+* and such a path is used.
+*/
+ return !has_include;
+}
+
+
+/*
* Read the patch text in buffer that extends for size bytes; stop
* reading after seeing a single patch (i.e. changes to a single file).
* Create fragments (i.e. patch hunks) and hang them to the given patch.
@@ -4145,44 +4188,6 @@ static int write_out_results(struct patch *list)
static struct lock_file lock_file;
-static struct string_list limit_by_name;
-static int has_include;
-static void add_name_limit(const char *name, int exclude)
-{
- struct string_list_item *it;
-
- it = string_list_append(limit_by_name, name);
- it-util = exclude ? NULL : (void *) 1;
-}
-
-static int use_patch(struct patch *p)
-{
- const char *pathname = p-new_name ? p-new_name : p-old_name;
- int i;
-
- /* Paths outside are not touched regardless of --include */
- if (0 prefix_length) {
- int pathlen = strlen(pathname);
- if (pathlen = prefix_length ||
- memcmp(prefix, pathname, prefix_length))
- return 0;
- }
-
- /* See if it matches any of exclude/include rule */
- for (i = 0; i limit_by_name.nr; i++) {
- struct string_list_item *it = limit_by_name.items[i];
- if (!fnmatch(it-string, pathname, 0))
- return (it-util != NULL);
- }
-
- /*
-* If we had any include, a path that does not match any rule is
-* not used. Otherwise, we saw bunch of exclude rules (or none)
-* and such a path is used.
-*/
- return !has_include;
-}
-
#define INACCURATE_EOF (10)
#define RECOUNT(11)
--
2.1.0-rc1-209-g4e1b551
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] apply: use the right attribute for paths in non-Git patches
We parse each patchfile and find to what path it applies to and then
use that name to consult the attribute system to find what
whitespace rules to be applied, and also which target file (either
in the working tree or in the index) to replay the changes the patch
represents.
A non-Git patch is taken as relative to the current working
directory, and the prefix_patches() helper function introduced in
56185f49 (git-apply: require -pn when working in a subdirectory.,
2007-02-19) is used to prepend the prefix to these names found in
the patch input.
However, this prepending of the prefix to the pathnames is done
after the patch is fully parsed and affects only what target files
are patched. Because the attributes are checked against the names
found in the patch during the parsing, not against the final path,
whitespace checks that are done during parsing end up using
attributes for a wrong path.
Because a Git-generated patch records the full path to the target
with -p$n prefix (e.g. a/ and b/) and we apply it relative to the
top of the working tree, prefix_patches() is a no-op and this
problem does not manifest for them.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 41 +++--
t/t4119-apply-config.sh | 17 +
2 files changed, 36 insertions(+), 22 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 6013e19..4270cde 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1920,6 +1920,23 @@ static int parse_binary(char *buffer, unsigned long
size, struct patch *patch)
return used;
}
+static void prefix_one(char **name)
+{
+ char *old_name = *name;
+ if (!old_name)
+ return;
+ *name = xstrdup(prefix_filename(prefix, prefix_length, *name));
+ free(old_name);
+}
+
+static void prefix_patch(struct patch *p)
+{
+ if (!prefix || p-is_toplevel_relative)
+ return;
+ prefix_one(p-new_name);
+ prefix_one(p-old_name);
+}
+
/*
* Read the patch text in buffer that extends for size bytes; stop
* reading after seeing a single patch (i.e. changes to a single file).
@@ -1935,6 +1952,8 @@ static int parse_chunk(char *buffer, unsigned long size,
struct patch *patch)
if (offset 0)
return offset;
+ prefix_patch(patch);
+
patch-ws_rule = whitespace_rule(patch-new_name
? patch-new_name
: patch-old_name);
@@ -4164,26 +4183,6 @@ static int use_patch(struct patch *p)
return !has_include;
}
-
-static void prefix_one(char **name)
-{
- char *old_name = *name;
- if (!old_name)
- return;
- *name = xstrdup(prefix_filename(prefix, prefix_length, *name));
- free(old_name);
-}
-
-static void prefix_patches(struct patch *p)
-{
- if (!prefix || p-is_toplevel_relative)
- return;
- for ( ; p; p = p-next) {
- prefix_one(p-new_name);
- prefix_one(p-old_name);
- }
-}
-
#define INACCURATE_EOF (10)
#define RECOUNT(11)
@@ -4209,8 +4208,6 @@ static int apply_patch(int fd, const char *filename, int
options)
break;
if (apply_in_reverse)
reverse_patches(patch);
- if (prefix)
- prefix_patches(patch);
if (use_patch(patch)) {
patch_stats(patch);
*listp = patch;
diff --git a/t/t4119-apply-config.sh b/t/t4119-apply-config.sh
index 3d0384d..be325fa 100755
--- a/t/t4119-apply-config.sh
+++ b/t/t4119-apply-config.sh
@@ -159,4 +159,21 @@ test_expect_success 'same but with traditional patch input
of depth 2' '
check_result sub/file1
'
+test_expect_success 'in subdir with traditional patch input' '
+ cd $D
+ git config apply.whitespace strip
+ cat .gitattributes -EOF
+ /* whitespace=blank-at-eol
+ sub/* whitespace=-blank-at-eol
+ EOF
+ rm -f sub/file1
+ cp saved sub/file1
+ git update-index --refresh
+
+ cd sub
+ git apply ../gpatch.file
+ echo B expect
+ test_cmp expect file1
+'
+
test_done
--
2.1.0-rc1-209-g4e1b551
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] apply: omit ws check for excluded paths
Whitespace breakages are checked while the patch is being parsed. Disable them at the beginning of parse_chunk(), where each individual patch is parsed, immediately after we learn what path the patch applies to and before we start parsing the changes. One may naively think we should be able to not just skip the whitespace checks but simply fast-forward to the next patch without doing anything once use_patch() tells us that this patch is not going to be used. But in reality we cannot really skip much of the parsing, primarily because parsing @@ -k,l +m,n @@ lines and counting the input lines is how we determine the boundaries of individual patches. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 9 ++--- t/t4124-apply-ws-rule.sh | 11 +++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index bf075cc..13319e8 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1997,9 +1997,12 @@ static int parse_chunk(char *buffer, unsigned long size, struct patch *patch) prefix_patch(patch); - patch-ws_rule = whitespace_rule(patch-new_name -? patch-new_name -: patch-old_name); + if (!use_patch(patch)) + patch-ws_rule = 0; + else + patch-ws_rule = whitespace_rule(patch-new_name +? patch-new_name +: patch-old_name); patchsize = parse_single_patch(buffer + offset + hdrsize, size - offset - hdrsize, patch); diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index 5d0c598..c6474de 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -512,4 +512,15 @@ test_expect_success 'whitespace=fix to expand' ' git -c core.whitespace=tab-in-indent apply --whitespace=fix patch ' +test_expect_success 'whitespace check skipped for excluded paths' ' + git config core.whitespace blank-at-eol + used + unused + git add used unused + echo used used + echo unused unused + git diff-files -p used unused patch + git apply --include=used --stat --whitespace=error patch +' + test_done -- 2.1.0-rc1-209-g4e1b551 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
Jakub Narębski jna...@gmail.com writes: There was (long time ago) a long thread about idea of adding some kind of 'weak' references (links), 'weakparent' that can be automatically used by Git but do not pollute the commit message, and do not affect reachability calculations. Ultimately it went nowhere (as you can see) - there were many problems. For example: how it would work for reverts and rebases? Perhaps some digging in the list archive before typing is in order. This may be a good starting point. http://thread.gmane.org/gmane.comp.version-control.git/46770/focus=46799 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cherry picking and merge
Junio C Hamano gits...@pobox.com writes: Jakub Narębski jna...@gmail.com writes: There was (long time ago) a long thread about idea of adding some kind of 'weak' references (links), 'weakparent' that can be automatically used by Git but do not pollute the commit message, and do not affect reachability calculations. Ultimately it went nowhere (as you can see) - there were many problems. For example: how it would work for reverts and rebases? Perhaps some digging in the list archive before typing is in order. This may be a good starting point. http://thread.gmane.org/gmane.comp.version-control.git/46770/focus=46799 Here is another. http://thread.gmane.org/gmane.comp.version-control.git/19126/focus=19149 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
Jonathan Nieder jrnie...@gmail.com writes: 2. Submodules aware of their superproject and of the parent's branches. In other words, submodules would act as thought under refs/ they had a symlink parent - ../../../refs So you could do git checkout --recurse-submodules master cd path/to/submodule git checkout parent/heads/next This would avoid danger from git gc in submodules and would get rid of most of the motivation for named branches in the submodule, I'd think. Are you assuming that they share their object stores? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: 2. Submodules aware of their superproject and of the parent's branches. In other words, submodules would act as though under refs/ they had a symlink parent - ../../../refs So you could do git checkout --recurse-submodules master cd path/to/submodule git checkout parent/heads/next This would avoid danger from git gc in submodules and would get rid of most of the motivation for named branches in the submodule, I'd think. Are you assuming that they share their object stores? No. The 'symlink' thing is a think-o. (When trying to explain the idea I ended up oversimplifying and speaking nonsense.) What I wanted to say is that parent/heads/next would be a way to refer from the submodule to the same commit as refs/heads/next:path/to/submodule refers to in the parent. Thanks for clarifying, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/23] Enable options --signoff, --reset-author for pick, reword
Hi List, this is the second reroll of the patch series fixing corner-case bugs regarding empty commits, commits with no log message and root commits, providing a uniform implementation of the to-do list commands using the `do_pick` interface. This reroll includes the following list of changes to PATCH v1 from last week, mostly induced by Jeff's comments. However, I didn't get around the most recent discussion from tonight yet. - new tests: allow replaying commits with empty log messages - coverage of all to-do list commands - new patch: allow squashing empty commits without complaints - do not complain about an empty squash commit unless it is the final one and --keep-empty is not specified on the command line - new tests: allow rewording empty commits without complaints - coverage of all to-do list commands - new patch: hide interactive command messages in verbose mode - make it possible to launch an editor inside 'output' - new patch: allow disabling pre-commit and commit-msg separately - add options --no-pre-commit and --no-commit-msg to git-commit - redefine --no-verify as synonym for the above two - new patch: squash skips commit-msg hook - run commit-msg hook for reworded _and_ squashed commits - a change to 'test_commit' options and 'fake_editor' debug output Thanks for your time and reviews, Fabian Fabian Ruch (23): rebase -i: allow replaying commits with empty log messages rebase -i: allow squashing empty commits without complaints rebase -i: allow rewording empty commits without complaints rebase -i: hide interactive command messages in verbose mode rebase -i: failed reword prints redundant error message commit: allow disabling pre-commit and commit-msg separately rebase -i: squash skips commit-msg hook rebase -i: reword executes pre-commit hook on interim commit rebase -i: teach do_pick the option --edit rebase -i: implement reword in terms of do_pick rebase -i: log the replay of root commits rebase -i: root commits are replayed with an unnecessary option rebase -i: commit only once when rewriting picks rebase -i: do not die in do_pick rebase -i: teach do_pick the option --amend rebase -i: teach do_pick the option --file rebase -i: prepare for squash in terms of do_pick --amend rebase -i: implement squash in terms of do_pick rebase -i: explicitly distinguish replay commands and exec tasks rebase -i: parse to-do list command line options rebase -i: teach do_pick the option --reset-author rebase -i: teach do_pick the option --signoff rebase -i: enable options --signoff, --reset-author for pick, reword Documentation/git-commit.txt | 8 +- builtin/commit.c | 32 - git-rebase--interactive.sh| 288 ++ git-rebase.sh | 12 +- t/lib-rebase.sh | 8 +- t/t3404-rebase-interactive.sh | 234 -- t/t3406-rebase-message.sh | 18 +++ t/t3412-rebase-root.sh| 16 +++ t/t7503-pre-commit-hook.sh| 65 -- t/t7504-commit-msg-hook.sh| 85 ++--- t/test-lib-functions.sh | 23 +++- 11 files changed, 680 insertions(+), 109 deletions(-) -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/23] rebase -i: allow replaying commits with empty log messages
git-rebase--interactive handles empty log messages inconsistently
between enabled and disabled fast-forwards. By default, commits with
empty log messages are rebased successfully like in non-interactive
mode. In contrast, the `--no-ff` option aborts the replay of such
commits.
In line with not verifying rebased commits and behaving like
git-rebase for `pick` lines, use the `--allow-empty-message` option
to replay commits. Root commits are replayed by recreating them in
`do_pick` using git-commit and all other commits are replayed using
git-cherry-pick in `pick_one`. Apply the option, understood by both
git-commit and git-cherry-pick, at the respective sites.
In case of `reword` and `squash`, continue to abort the rebase if the
_resulting_ commit would have no commit message. The rationale behind
this default is that patches and their log messages should be
verified at least once. For unchanged commits this is assumed to have
happened according to the author's standards when she created the
commits for the first time. While the empty log message can always be
kept in place by editing and resuming the aborted rebase, a debatable
alternative could be to teach git-rebase--interactive the option
`--allow-empty-message` for disabling complaints about empty log
messages even in changed commits.
The `fixup` case is different again because it throws away the second
commit's log message and uses the first log message for the changed
commit. Do not abort the rebase if that message is empty either since
it is assumed to have been verified already.
The remaining to-do list command `edit` is handled just like `pick`
for this matter, because git-rebase--interactive replays the named
commit without changes before the rebase is interrupted and the user
can make her changes to the replayed commit.
Add tests. In particular, design the `squash`-specific test case such
that it involves interim commits and `fixup` steps. Interim commits
should not trigger failures themselves and `fixup` steps should not
let git-rebase--interactive forget that it is still dealing with a
`squash` result.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 10 ++
t/t3404-rebase-interactive.sh | 38 ++
t/t3412-rebase-root.sh| 16
3 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b64dd28..3222bf6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -249,7 +249,7 @@ pick_one () {
test -d $rewritten
pick_one_preserving_merges $@ return
- output eval git cherry-pick \
+ output eval git cherry-pick --allow-empty-message \
${gpg_sign_opt:+$(git rev-parse --sq-quote
$gpg_sign_opt)} \
$strategy_args $empty_args $ff $@
}
@@ -363,7 +363,7 @@ pick_one_preserving_merges () {
echo $sha1 $(git rev-parse HEAD^0)
$rewritten_list
;;
*)
- output eval git cherry-pick \
+ output eval git cherry-pick --allow-empty-message \
${gpg_sign_opt:+$(git rev-parse --sq-quote
$gpg_sign_opt)} \
$strategy_args $@ ||
die_with_patch $sha1 Could not pick $sha1
@@ -549,7 +549,8 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- do_with_author output git commit --amend --no-verify -F
$squash_msg \
+ do_with_author output git commit --allow-empty-message \
+ --amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
;;
@@ -557,7 +558,8 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --amend --no-verify
-F $fixup_msg \
+ do_with_author git commit --allow-empty-message
\
+ --amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed2..9c71835 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,42 @@ test_expect_success 'short SHA-1 collide' '
)
'
+test_expect_success 'setup commits with empty commit log
[PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
The to-do list commands `squash` and `fixup` apply the changes
introduced by the named commit to the tree but instead of creating
a new commit on top of the current head it replaces the previous
commit with a new commit that records the updated tree. If the
result is an empty commit git-rebase stops with the error message
You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.
This message is not very helpful because neither does git-rebase
support an option `--allow-empty` nor does the messages say how to
resume the rebase. Firstly, change the error message to
The squash result is empty and --keep-empty was not specified.
You can remove the squash commit now with
git reset HEAD^
Once you are down, run
git rebase --continue
If the user wishes to squash a sequence of commits into one
commit, f. i.
pick A
squash Revert A
squash A'
, it does not matter for the end result that the first squash
result, or any sub-sequence in general, is going to be empty. The
squash message is not affected at all by which commits are created
and only the commit created by the last line in the sequence will
end up in the final history. Secondly, print the error message
only if the whole squash sequence produced an empty commit.
Lastly, since an empty squash commit is not a failure to rewrite
the history as planned, issue the message above as a mere warning
and interrupt the rebase with the return value zero. The
interruption should be considered as a notification with the
chance to undo it on the spot. Specifying the `--keep-empty`
option tells git-rebase to keep empty squash commits in the
rebased history without notification.
Add tests.
Reported-by: Peter Krefting pe...@softwolves.pp.se
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
Peter Krefting is cc'd as the author of the bug report Confusing
error message in rebase when commit becomes empty discussed on the
mailing list in June. Phil Hord and Jeff King both participated in
the problem discussion which ended with two proposals by Jeff.
Jeff King writes:
1. Always keep such empty commits. A user who is surprised by them
being empty can then revisit them. Or drop them by doing another
rebase without --keep-empty.
2. Notice ourselves that the end-result of the whole squash is an
empty commit, and stop to let the user deal with it.
This patch chooses the second alternative. Either way seems OK. The
crucial consensus of the discussion was to silently throw away empty
interim commits.
Fabian
git-rebase--interactive.sh| 20 +++---
t/t3404-rebase-interactive.sh | 62 +++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3222bf6..8820eac 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -549,7 +549,7 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- do_with_author output git commit --allow-empty-message \
+ do_with_author output git commit --allow-empty-message
--allow-empty \
--amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
@@ -558,18 +558,32 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --allow-empty-message
\
+ do_with_author git commit --allow-empty-message
--allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author git commit --allow-empty --amend
--no-verify -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
fi
rm -f $squash_msg $fixup_msg
+ if test -z $keep_empty is_empty_commit HEAD
+ then
+ echo $sha1 $state_dir/stopped-sha
+
[PATCH v2 03/23] rebase -i: allow rewording empty commits without complaints
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If `--keep-empty` is
passed as option to git-rebase--interactive, empty commits ought to
be replayed without complaints. However, if the users chooses to
reword an empty commit by changing the respective to-do list entry
from
pick fa1afe1 Empty commit
to
reword fa1afe1 Empty commit
, then git-rebase--interactive suddenly fails to replay the empty
commit. This is especially counterintuitive because `reword` is
thought of as a `pick` that alters the log message in some way but
nothing more and the unchanged to-do list entry would not fail.
Handle `reword` by cherry-picking the named commit and editing the
log message using
git commit --allow-empty --amend
instead of
git commit --amend.
Add test.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 2 +-
t/t3404-rebase-interactive.sh | 24
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8820eac..89ef5e2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,7 +504,7 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
+ git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
warn This is most likely due to an empty commit
message, or the pre-commit hook
warn failed. If the pre-commit hook failed, you may
need to resolve the issue before
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index a95cb2a..3e64280 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -75,6 +75,30 @@ test_expect_success 'rebase --keep-empty' '
test_line_count = 6 actual
'
+test_expect_success 'rebase --keep-empty (reword)' '
+ git checkout -b reword-emptybranch emptybranch
+ set_fake_editor
+ FAKE_LINES=1 reword 2 git rebase --keep-empty -i HEAD~2
+ git log --oneline actual
+ test_line_count = 6 actual
+'
+
+test_expect_success 'rebase --keep-empty (fixup)' '
+ git checkout -b fixup-emptybranch emptybranch
+ set_fake_editor
+ FAKE_LINES=1 fixup 2 git rebase --keep-empty -i HEAD~2
+ git log --oneline actual
+ test_line_count = 5 actual
+'
+
+test_expect_success 'rebase --keep-empty (squash)' '
+ git checkout -b squash-emptybranch emptybranch
+ set_fake_editor
+ FAKE_LINES=1 squash 2 git rebase --keep-empty -i HEAD~2
+ git log --oneline actual
+ test_line_count = 5 actual
+'
+
test_expect_success 'rebase -i with the exec command' '
git checkout master
(
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/23] rebase -i: failed reword prints redundant error message
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. If the edited log
message is empty or is found ill-formatted by one of the commit
hooks, git-rebase--interactive prints three error messages to the
console.
1. The git-commit output, which contains all the output from hook
scripts.
2. A rebase diagnosis saying at which task on the to-do list it
got stuck.
3. Generic presumptions about what could have triggered the
error.
The third message contains redundant information and does not add any
enlightenment either, which makes the output unnecessarily longish
and different from the other command's output. For instance, this is
what the output looks like if the log message is empty (contains
duplicate Signed-off-by lines).
(1.) Aborting commit due to empty commit message. (Duplicate Signed-off-by
lines.)
(2.) Could not amend commit after successfully picking fa1afe1... Some
change
(3.) This is most likely due to an empty commit message, or the pre-commit
hook
failed. If the pre-commit hook failed, you may need to resolve the
issue before
you are able to reword the commit.
Discard the third message.
It is true that a failed hook script might not output any diagnosis
but then the generic message is not of much help either. Since this
lack of information affects the built-in git commands for commit,
merge and cherry-pick first, the solution would be to keep track of
the failed hooks in their output so that the user knows which of her
hooks require improvement.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 3 ---
1 file changed, 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5dfdf13..3ee13c2 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -506,9 +506,6 @@ do_next () {
do_pick $sha1 $rest
output git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
- warn This is most likely due to an empty commit
message, or the pre-commit hook
- warn failed. If the pre-commit hook failed, you may
need to resolve the issue before
- warn you are able to reword the commit.
exit_with_patch $sha1 1
}
record_in_rewritten $sha1
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/23] rebase -i: hide interactive command messages in verbose mode
git-rebase--interactive prints summary messages of the commits it
creates in the final history only if the `--verbose` option is
specified by the user and suppresses them otherwise. This behaviour
is implemented by wrapping git-commit calls in a shell function named
`output` which redirects stderr to stdout, captures stdout in a shell
variable and ignores its contents unless the command exits with an
error status.
The command lines used to implement the to-do list commands `reword`
and `squash` print diagnostic messages even in non-verbose mode. The
reason for this inconsistency is that both commands launch the log
message editor which usually requires a working terminal attached to
stdin. Temporarily redirect the editor output to a third file
descriptor in order to ship it around the capture stream. Wrap the
respective git-commit command lines in `output`.
fake_editor prints the to-do list before and after applying the
`FAKE_LINES` rewrite rules to it. Redirect this debug output to
stderr so that it does not interfere with the git-rebase status
output. Add test.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 9 +
git-rebase.sh | 12 ++--
t/lib-rebase.sh | 8
t/t3404-rebase-interactive.sh | 18 ++
t/t3406-rebase-message.sh | 18 ++
5 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 89ef5e2..5dfdf13 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,7 +504,7 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
+ output git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
warn This is most likely due to an empty commit
message, or the pre-commit hook
warn failed. If the pre-commit hook failed, you may
need to resolve the issue before
@@ -558,14 +558,14 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --allow-empty-message
--allow-empty \
+ do_with_author output git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --allow-empty --amend
--no-verify -F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author output git commit --allow-empty
--amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
fi
@@ -923,6 +923,8 @@ EOF
;;
esac
+mkdir -p $state_dir || die Could not create temporary $state_dir
+
git var GIT_COMMITTER_IDENT /dev/null ||
die You need to set your committer info first
@@ -938,7 +940,6 @@ then
fi
orig_head=$(git rev-parse --verify HEAD) || die No HEAD?
-mkdir -p $state_dir || die Could not create temporary $state_dir
: $state_dir/interactive || die Could not mark as interactive
write_basic_state
diff --git a/git-rebase.sh b/git-rebase.sh
index 55da9db..f90541e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -131,9 +131,17 @@ write_basic_state () {
output () {
case $verbose in
'')
- output=$($@ 21 )
+ cat $state_dir/editor.sh EOF
+#!/bin/sh
+$(git var GIT_EDITOR) \$@ 3
+EOF
+ chmod +x $state_dir/editor.sh
+ (
+ export GIT_EDITOR=\$state_dir/editor.sh\
+ $@ 31 $state_dir/output 21
+ )
status=$?
- test $status != 0 printf %s\n $output
+ test $status != 0 cat $state_dir/output
return $status
;;
*)
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 6bd2522..0cd1193 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -41,8 +41,8 @@ set_fake_editor () {
test -z $FAKE_LINES exit
grep -v '^#' $1 $1.tmp
rm -f $1
- echo 'rebase -i script before editing:'
- cat $1.tmp
+ echo 'rebase -i script before editing:' 2
+ cat $1.tmp 2
action=pick
[PATCH v2 08/23] rebase -i: reword executes pre-commit hook on interim commit
The to-do list command `reword` replays a commit like `pick` but lets
the user also edit the commit's log message. This happens in two
steps. Firstly, the named commit is cherry-picked. Secondly, the
commit created in the first step is amended using an unchanged index
to edit the log message. The pre-commit hook is meant to verify the
changes introduced by a commit (for instance, catching inserted
trailing white space). Since the committed tree is not changed
between the two steps and we do not verify rebased patches, do not
execute the pre-commit hook in the second step.
Specify the git-commit option `--no-pre-commit` to disable the
pre-commit hook when editing the log message. The commit-msg hook
will still be executed to verify the edited commit log message. As
before, if the hook finds the new log message ill-formatted, the
rebase will be interrupted with the unchanged commit replayed and the
new log message in `$GIT_DIR/COMMIT_EDITMSG`.
Add tests.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh| 2 +-
t/t3404-rebase-interactive.sh | 14 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 97386aa..edc323d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -504,7 +504,7 @@ do_next () {
mark_action_done
do_pick $sha1 $rest
- output git commit --allow-empty --amend --no-post-rewrite
${gpg_sign_opt:+$gpg_sign_opt} || {
+ output git commit --allow-empty --amend --no-post-rewrite
--no-pre-commit ${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking
$sha1... $rest
exit_with_patch $sha1 1
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index bc2dda1..3dac022 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -672,6 +672,13 @@ test_expect_success 'setup failing pre-commit' '
test_commit --no-verify pre-commit-violated-2
'
+test_expect_success 'reword a commit violating pre-commit' '
+ git checkout -b reword-violating-pre-commit violating-pre-commit
+ test_when_finished reset_rebase
+ set_fake_editor
+ env FAKE_LINES=reword 1 git rebase -i master
+'
+
test_expect_success 'squash a commit violating pre-commit' '
git checkout -b squash-violating-pre-commit violating-pre-commit
test_when_finished reset_rebase
@@ -715,6 +722,13 @@ test_expect_success 'rebase a commit violating commit-msg'
'
FAKE_LINES=1 git rebase -i master
'
+test_expect_success 'reword a commit violating commit-msg' '
+ git checkout -b reword-violating-commit-msg violating-commit-msg
+ test_when_finished reset_rebase
+ set_fake_editor
+ test_must_fail env FAKE_LINES=reword 1 git rebase -i master
+'
+
test_expect_success 'squash a commit violating commit-msg' '
git checkout -b squash-violating-commit-msg violating-commit-msg
set_fake_editor
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/23] commit: allow disabling pre-commit and commit-msg separately
Introduce the git-commit command line options `--no-pre-commit` and
`--no-commit-msg` to disable the pre-commit and commit-msg hooks,
respectively. Make `--no-verify` a synonym for specifying both at the
same time.
This change is motivated by an internal usage of git-commit in
git-rebase--interactive to disable pre-commit while keeping
commit-msg enabled when rewording a commit.
Make `test_commit` forward unknown options to git-commit instead of
teaching it all possible options. In order to support leading double
dashes in `message`, stop interpreting `test_commit` arguments
following a `--` argument as options. This wasn't a problem before
because the first unknown option would be used as `message`.
Allow disabling tag creation to avoid name clashes when using
`test_commit` with the same arguments several times from the same
test suite. By default, `test_commit` tags successful commits using
git-tag for easy reference. The `--notag` option skips this step.
Add tests.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Documentation/git-commit.txt | 8 -
builtin/commit.c | 32 ++---
t/t7503-pre-commit-hook.sh | 65 -
t/t7504-commit-msg-hook.sh | 85 ++--
t/test-lib-functions.sh | 23
5 files changed, 176 insertions(+), 37 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..28a2c5c 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -158,7 +158,7 @@ OPTIONS
-n::
--no-verify::
- This option bypasses the pre-commit and commit-msg hooks.
+ A synonym for `--no-pre-commit --no-commit-msg`.
See also linkgit:githooks[5].
--allow-empty::
@@ -238,6 +238,12 @@ You should understand the implications of rewriting
history if you
amend a commit that has already been published. (See the RECOVERING
FROM UPSTREAM REBASE section in linkgit:git-rebase[1].)
+--no-pre-commit::
+ This option bypasses the pre-commit hook.
+
+--no-commit-msg::
+ This option bypasses the commit-msg hook.
+
--no-post-rewrite::
Bypass the post-rewrite hook.
diff --git a/builtin/commit.c b/builtin/commit.c
index 5ed6036..dfd354e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -98,12 +98,27 @@ static char *edit_message, *use_message;
static char *fixup_message, *squash_message;
static int all, also, interactive, patch_interactive, only, amend, signoff;
static int edit_flag = -1; /* unspecified */
-static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
+static int quiet, verbose, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
static char *sign_commit;
/*
+ * The verify variable is interpreted as a bitmap of enabled commit
+ * verification hooks according to the legend below.
+ *
+ * By default, the pre-commit and commit-msg hooks are enabled. This
+ * is represented by both the PRE_COMMIT and COMMIT_MSG bits being
+ * set.
+ *
+ * The bitmap is changed through the command line options
+ * --no-verify, --no-pre-commit and --no-commit-msg.
+ */
+#define PRE_COMMIT (10)
+#define COMMIT_MSG (11)
+static int verify = PRE_COMMIT | COMMIT_MSG;
+
+/*
* The default commit message cleanup mode will remove the lines
* beginning with # (shell comments) and leading and trailing
* whitespaces (empty lines or containing only whitespaces)
@@ -661,7 +676,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
- if (!no_verify run_commit_hook(use_editor, index_file, pre-commit,
NULL))
+ if (verify PRE_COMMIT
+ run_commit_hook(use_editor, index_file, pre-commit, NULL))
return 0;
if (squash_message) {
@@ -962,7 +978,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
}
}
- if (!no_verify
+ if (verify COMMIT_MSG
run_commit_hook(use_editor, index_file, commit-msg,
git_path(commit_editmsg), NULL)) {
return 0;
}
@@ -1590,7 +1606,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, interactive, interactive, N_(interactively add
files)),
OPT_BOOL('p', patch, patch_interactive, N_(interactively
add changes)),
OPT_BOOL('o', only, only, N_(commit only specified files)),
- OPT_BOOL('n', no-verify, no_verify, N_(bypass pre-commit
hook)),
+ OPT_NEGBIT('n', no-verify, verify,
+ N_(synonym for --no-pre-commit --no-commit-msg),
+ PRE_COMMIT | COMMIT_MSG),
OPT_BOOL(0, dry-run, dry_run, N_(show what would be
committed)),
[PATCH v2 09/23] rebase -i: teach do_pick the option --edit
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement the to-do list command `pick`. To cater for
the different pick behaviours (like `reword`), `do_pick` accepts
several options not only from the git-cherry-pick but also the
git-commit interface. Add the common option `--edit` to let the user
edit the log message of the named commit.
Loop over `$@` to parse the `do_pick` arguments. Assign the local
variable `edit` if one of the options is `--edit` so that the
remainder of `do_pick` can easily check whether the client code asked
to edit the commit message. If one of the options is unknown, mention
it on the console and `die`. Break the loop on the first non-option
and do some sanity checking to ensure that there exactly two
non-options, which are interpreted by the remainder as `commit` and
`title` like before.
`do_pick` ought to act as a wrapper around `cherry-pick`.
Unfortunately, it cannot just forward `--edit` to the `cherry-pick`
command line. The assembled command line is executed within a command
substitution for controlling the verbosity of `rebase--interactive`.
Passing `--edit` would either hang the terminal or clutter the
substituted command output with control sequences. Execute the
`reword` code from `do_next` instead if the option `--edit` is
specified.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 43 +++
1 file changed, 43 insertions(+)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index edc323d..aed2f93 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -461,7 +461,42 @@ record_in_rewritten() {
esac
}
+# Apply the changes introduced by the given commit to the current head.
+#
+# do_pick [--edit] commit title
+#
+# Wrapper around git-cherry-pick.
+#
+# -e, --edit
+# After picking commit, open an editor and let the user edit the
+# commit message. The editor contents becomes the commit message of
+# the new head. This creates a fresh commit.
+#
+# commit
+# The commit to cherry-pick.
+#
+# title
+# The commit message title of commit. Used for information
+# purposes only.
do_pick () {
+ edit=
+ while test $# -gt 0
+ do
+ case $1 in
+ -e|--edit)
+ edit=y
+ ;;
+ -*)
+ die do_pick: unrecognized option -- $1
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done
+ test $# -ne 2 die do_pick: wrong number of arguments
+
if test $(git rev-parse HEAD) = $squash_onto
then
# Set the correct commit message and author info on the
@@ -483,6 +518,14 @@ do_pick () {
pick_one $1 ||
die_with_patch $1 Could not apply $1... $2
fi
+
+ if test -n $edit
+ then
+ output git commit --allow-empty --amend --no-post-rewrite
--no-pre-commit ${gpg_sign_opt:+$gpg_sign_opt} || {
+ warn Could not amend commit after successfully picking
$1... $2
+ exit_with_patch $1 1
+ }
+ fi
}
do_next () {
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/23] rebase -i: teach do_pick the option --amend
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement the to-do list commands `pick`, `reword`
and `edit`. To cater for the different pick behaviours (like
`squash`), `do_pick` accepts several options not only from the
git-cherry-pick but also the git-commit interface.
Add the option `--amend` from the git-commit interface to the options
pool of `do_pick`. It creates a new commit for the changes introduced
by the picked commit and the previous one. The previous commit is
then replaced with the new commit. If no other options are specified,
the log message of the previous commit is used.
Be careful when `--amend` is used to pick a root commit because HEAD
might point to the sentinel commit but there is still nothing to
amend. Be sure to initialize `amend` so that commits are squashed
even when git-rebase--interactive is interrupted for resolving
conflicts. It is not a mistake to do the initialization regardless of
any conflicts because `amend` is always cleared before the next to-do
item is processed.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d812bad..0871302 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,10 +464,16 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--edit] commit
+# do_pick [--amend] [--edit] commit
#
# Wrapper around git-cherry-pick.
#
+# --amend
+# After picking commit, replace the current head commit with a new
+# commit that also introduces the changes of commit.
+#
+# _This is not a git-cherry-pick option._
+#
# -e, --edit
# After picking commit, open an editor and let the user edit the
# commit message. The editor contents becomes the commit message of
@@ -489,6 +495,16 @@ do_pick () {
while test $# -gt 0
do
case $1 in
+ --amend)
+ if test $(git rev-parse HEAD) = $squash_onto || !
git rev-parse -q --verify HEAD /dev/null
+ then
+ warn do_pick: nothing to amend
+ return 2
+ fi
+ rewrite=y
+ rewrite_amend=y
+ git rev-parse --verify HEAD $amend
+ ;;
-e|--edit)
rewrite=y
rewrite_edit=y
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 20/23] rebase -i: parse to-do list command line options
Read in to-do list lines as
command args
instead of
command sha1 rest
so that to-do list command lines can specify additional arguments
apart from the commit hash and the log message title, which become
the non-options in `args`. Loop over `args`, put all options (an
argument beginning with a dash) in `opts`, stop the loop on the first
non-option and assign it to `sha1`. The loop does not know the
options it parses so that options that take an argument themselves
are not supported at the moment. Neither are options that contain
spaces because the shell expansion of `args` in `do_next` interprets
white space characters as argument separator, that is a command line
like
pick --author A U Thor fa1afe1 Some change
is parsed as the pick command
pick --author
and the commit hash
A
which obviously results in an unknown revision error. For the sake of
completeness, in the example above the message title variable `rest`
is assigned the string 'U Thor fa1afe1 Some change' (without the
single quotes).
Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment. Cleanly break the `do_next` loop by assigning the special
value 'unknown' to the local variable `command`, which triggers the
unknown command case in `do_cmd`.
The to-do list is also parsed when the commit hashes are translated
between long and short format before and after the to-do list is
edited. Apply the same procedure as in `do_cmd` with the exception
that we only care about where the options stop and the commit hash
begins. Do not reject any options when transforming the commit
hashes.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 49 ++
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ecd4d7..da435cb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -577,8 +577,26 @@ do_pick () {
do_replay () {
command=$1
- sha1=$2
- rest=$3
+ shift
+
+ opts=
+ while test $# -gt 0
+ do
+ case $1 in
+ -*)
+ warn Unknown option: $1
+ command=unknown
+ ;;
+ *)
+ break
+ ;;
+ esac
+ opts=$opts $(git rev-parse --sq-quote $1)
+ shift
+ done
+ sha1=$1
+ shift
+ rest=$*
case $command in
pick|p)
@@ -675,7 +693,7 @@ do_replay () {
do_next () {
rm -f $msg $author_script $amend || exit
- read -r command sha1 rest $todo
+ read -r command args $todo
case $command in
$comment_char*|''|noop)
@@ -720,7 +738,7 @@ do_next () {
fi
;;
*)
- do_replay $command $sha1 $rest
+ do_replay $command $args
;;
esac
test -s $todo return
@@ -800,19 +818,34 @@ skip_unnecessary_picks () {
}
transform_todo_ids () {
- while read -r command rest
+ while read -r command args
do
case $command in
$comment_char* | exec)
# Be careful for oddball commands like 'exec'
# that do not have a SHA-1 at the beginning of $rest.
+ newargs=\ $args
;;
*)
- sha1=$(git rev-parse --verify --quiet $@ ${rest%% *})
- rest=$sha1 ${rest#* }
+ newargs=
+ sha1=
+ for arg in $args
+ do
+ case $arg in
+ -*)
+ newargs=$newargs $arg
+ ;;
+ *)
+ test -z $sha1
+ sha1=$(git rev-parse --verify
--quiet $@ $arg)
+ arg=$sha1
+ newargs=$newargs $arg
+ ;;
+ esac
+ done
;;
esac
- printf '%s\n' $command${rest:+ }$rest
+ printf '%s\n' $command$newargs
done $todo $todo.new
mv -f $todo.new $todo
}
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/23] rebase -i: teach do_pick the option --file
`do_pick` is the git-cherry-pick wrapper in git-rebase--interactive
that is used to implement the to-do list command `pick`, `reword` and
`edit`. To cater for the different pick behaviours (like `squash`),
`do_pick` accepts several options not only from the git-cherry-pick
but also the git-commit interface.
Add the option `--file` from the git-commit interface to the options
pool of `do_pick`. It expects an argument itself which is interpreted
as a file path and takes the commit message from the given file. If
`--file` is passed to `do_pick`, assign the given file path to the
local variable `rewrite_message` and relay the option
--file $rewrite_message
to the git-commit command line which creates the commit.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0871302..0fbf773 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -464,7 +464,7 @@ record_in_rewritten() {
# Apply the changes introduced by the given commit to the current head.
#
-# do_pick [--amend] [--edit] commit
+# do_pick [--amend] [--file file] [--edit] commit
#
# Wrapper around git-cherry-pick.
#
@@ -474,6 +474,12 @@ record_in_rewritten() {
#
# _This is not a git-cherry-pick option._
#
+# -F file, --file file
+# Take the commit message from the given file. This creates a fresh
+# commit.
+#
+# _This is not a git-cherry-pick option._
+#
# -e, --edit
# After picking commit, open an editor and let the user edit the
# commit message. The editor contents becomes the commit message of
@@ -492,6 +498,7 @@ do_pick () {
rewrite=
rewrite_amend=
rewrite_edit=
+ rewrite_message=
while test $# -gt 0
do
case $1 in
@@ -505,6 +512,16 @@ do_pick () {
rewrite_amend=y
git rev-parse --verify HEAD $amend
;;
+ -F|--file)
+ if test $# -eq 0
+ then
+ warn do_pick: option --file specified but no
file given
+ return 2
+ fi
+ rewrite=y
+ rewrite_message=$2
+ shift
+ ;;
-e|--edit)
rewrite=y
rewrite_edit=y
@@ -553,6 +570,7 @@ do_pick () {
${allow_empty_message:+--allow-empty-message} \
${rewrite_amend:+--amend} \
${rewrite_edit:+--edit --commit-msg} \
+ ${rewrite_message:+--file $rewrite_message} \
${gpg_sign_opt:+$gpg_sign_opt} || return 3
fi
}
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
