From: Junio C Hamano <hidden> Date: 2016-06-15 22:44:31
Jörg Sommer [off-list ref] writes:
quoted hunk
This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
The decision to use references for the marks and not files like done with
the rewritten commits for preserve merges was made to ensure no commit
objects get lost if prune is started while (a long term) rebase is
running. This also unifies the checking of the validity of marks and
references by using rev-parse for it.
The usage of : as the sign for marks conforms with the tag sign of
fast-export and fast-import.
Signed-off-by: Jörg Sommer <redacted>
---
git-rebase--interactive.sh | 37 ++++++++++++++++++++++++++++++++++++-
t/t3404-rebase-interactive.sh | 17 +++++++++++++++++
2 files changed, 53 insertions(+), 1 deletions(-)
@@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and run'git rebase --continue'"exportGIT_CHERRY_PICK_HELP+mark_prefix=refs/rebase-marks/+ warn(){echo"$*">&2}
In practice nobody would "run" pack-refs during the rebase session, but I
have to wonder if it can be triggered to run as part of automated gc or
something, in which case this loop does not work as intended. It needs to
be rewritten using for-each-ref.
quoted hunk
@@ -244,6 +252,19 @@ peek_next_command () { sed -n "1s/ .*$//p" < "$TODO" }+mark_to_ref () {+ case "$1" in+ :[!/]*)+ # :/SOMETHING is a reference for the last commit whose+ # message starts with SOMETHING+ echo "$mark_prefix${1#:}"+ ;;
What was the conclusion of the mark-syntax discussion?
While I know the bang in ":[!negated]" is POSIX, I wonder if everybody's
shell we care about groks it.
Could people run this with the shell they care about being supported
(Solaris /bin/sh does not count) try this and yell loudly if you get
"matches" please? I know bash and dash are Ok, but I do not have easy
access to various flabours of BSDs (OSX included).
case ":/foo" in
:[!/]*) echo matches ;;
*) echo does not ;;
esac
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:31
Hi,
On Mon, 21 Apr 2008, Junio C Hamano wrote:
Jörg Sommer [off-list ref] writes:
quoted
@@ -244,6 +252,19 @@ peek_next_command () { sed -n "1s/ .*$//p" < "$TODO" }+mark_to_ref () {+ case "$1" in+ :[!/]*)+ # :/SOMETHING is a reference for the last commit whose+ # message starts with SOMETHING+ echo "$mark_prefix${1#:}"+ ;;
What was the conclusion of the mark-syntax discussion?
Well, I will probably work on something that is not as intrusive and
syntax-changing this week.
While I know the bang in ":[!negated]" is POSIX, I wonder if everybody's
shell we care about groks it.
The common way to do that in the git sources is
switch "$x" in
/*)
# do nothing
;;
*)
<bla>
;;
esac
and frankly, I do not see a reason to move away from that practice.
Especially since consistency in source code is better than inconsistency.
Ciao,
Dscho
From: Jörg Sommer <hidden> Date: 2016-06-15 22:44:31
Hi Junio,
Junio C Hamano schrieb am Mon 21. Apr, 22:32 (-0700):
Jörg Sommer [off-list ref] writes:
quoted
cleanup_before_quit () {
- rm -rf "$DOTEST"
+ rm -rf "$DOTEST" &&
+ for ref in "$GIT_DIR/$mark_prefix"*
+ do
+ test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
+ git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
+ return 1
+ done
In practice nobody would "run" pack-refs during the rebase session, but I
have to wonder if it can be triggered to run as part of automated gc or
something, in which case this loop does not work as intended. It needs to
be rewritten using for-each-ref.
What do you think about this version:
cleanup_before_quit () {
rm -rf "$DOTEST" &&
for ref in $(git for-each-ref --format='%(refname)' ${mark_prefix%/})
do
git update-ref -d "$ref" "$ref" || return 1
done
}
quoted
@@ -244,6 +252,19 @@ peek_next_command () { sed -n "1s/ .*$//p" < "$TODO" }+mark_to_ref () {+ case "$1" in+ :[!/]*)+ # :/SOMETHING is a reference for the last commit whose+ # message starts with SOMETHING+ echo "$mark_prefix${1#:}"+ ;;
What was the conclusion of the mark-syntax discussion?
Use the same as fast-import and fix fast-import. :)
I've posted a new version
[off-list ref]
Bye, Jörg.
--
Nutze die Talente, die du hast. Die Wälder wären sehr still,
wenn nur die begabtesten Vögel sängen. (Henry van Dyke)
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:31
Hi,
On Tue, 22 Apr 2008, Jörg Sommer wrote:
Junio C Hamano schrieb am Mon 21. Apr, 22:32 (-0700):
quoted
What was the conclusion of the mark-syntax discussion?
Use the same as fast-import and fix fast-import. :)
I strongly disagree.
Also with the conclusion that this was the conclusion.
fast-import is a fundamentally different thing compared to rebase -i. The
former should be very easy to write importers for, and therefore have a
very easy syntax from the _technical_ view, the latter should be usable by
people, and therefore have a very easy syntax from the _usability_ point
of view.
So I really hate the idea of introducing yet other marks when we already
have unique identifiers: the (abbreviated) commit names.
I guess the only way to prove that I am not wrong is to do it myself.
Sigh.
Ciao,
Dscho
From: Junio C Hamano <hidden> Date: 2016-06-15 22:44:31
Johannes Schindelin [off-list ref] writes:
So I really hate the idea of introducing yet other marks when we already
have unique identifiers: the (abbreviated) commit names.
Didn't I give you an example why commit object names are _not_ unique
identifiers already?
I also do not understand why you think 'mark' is ugly. I _suspect_ that
the machinery to read the TODO insns would need to keep a copy of what it
gave to the end user, compare it with what user edited to make sure the
user did not make nonsense insn sequence out of it, which Jörg's code
doesn't do (yet), and I suspect a simple rule like "you cannot move insns
across 'mark' boundary" would be sufficient for that check.
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:31
Hi,
On Tue, 22 Apr 2008, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
So I really hate the idea of introducing yet other marks when we already
have unique identifiers: the (abbreviated) commit names.
Didn't I give you an example why commit object names are _not_ unique
identifiers already?
By that reasoning, rebase -i cannot work anyway: it relies on the
abbreviated identifiers, not on anything else, for the "pick" command.
I also do not understand why you think 'mark' is ugly.
Is that not obvious? You _already_ have identifiers. And there you add
other ones. Only because the original idea of the -p implementation was
ignored.
Ciao,
Dscho
From: Junio C Hamano <hidden> Date: 2016-06-15 22:44:32
Johannes Schindelin [off-list ref] writes:
On Tue, 22 Apr 2008, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
So I really hate the idea of introducing yet other marks when we already
have unique identifiers: the (abbreviated) commit names.
Didn't I give you an example why commit object names are _not_ unique
identifiers already?
By that reasoning, rebase -i cannot work anyway: it relies on the
abbreviated identifiers, not on anything else, for the "pick" command.
No, read the message again and think for 5 minutes.
Picking the same commit twice does not make any sense, neither does
picking the resulting commit from an earlier operation in the same
sequencer run. Which means that the commit object name for 'pick' can
mean _only_ the pre-rewritten commit object, not 'the result of an earlier
operation that used that commit'. And you always pick on top of the
current (detached) HEAD.
Reset is different. You can reset either to the named commit to start
building from a known state that existed before the sequencer run started,
or reset to the result of pick (or merge) of the named commit, and your
proposal breaks down here, because you cannot tell between the two.
To rebuild this history on top of a commit O' elsewhere:
O---A---B
\ \
D---E---F---G
/
X
you would need to:
pick A
pick B
reset <<to the state after "pick A">>
pick D
merge <<the state after "pick B">>
pick F
merge X (taken from somebody else)
and the syntax proposed to express <<the above part>> can either be your
"the result of the last operation that used the named commit", which is
simple in some cases, or "named commit, be it with mark or standard sha-1
expression".
Introducing a 'mark' insn to mark the previous result you may want to go
back to is one way to solve this without ambiguity. Then abbreviated
object name won't have to be mapped as in your proposal.
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:44:32
Hi,
On Mon, 28 Apr 2008, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
On Tue, 22 Apr 2008, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
So I really hate the idea of introducing yet other marks when we already
have unique identifiers: the (abbreviated) commit names.
Didn't I give you an example why commit object names are _not_ unique
identifiers already?
By that reasoning, rebase -i cannot work anyway: it relies on the
abbreviated identifiers, not on anything else, for the "pick" command.
No, read the message again and think for 5 minutes.
pick abcdefg
pick pqrstuv
Now imagine that pqrstuv is a unique commit name _before_ cherry-picking
abcdefg, but not _after_ it. Unlikely? Yes. Impossible? No.
Ciao,
Dscho