* [PATCH] Add git-save script
@ 2007-06-23 13:02 Nanako Shiraishi
2007-06-23 15:05 ` Johannes Schindelin
2007-06-23 20:15 ` Junio C Hamano
0 siblings, 2 replies; 12+ messages in thread
From: Nanako Shiraishi @ 2007-06-23 13:02 UTC (permalink / raw)
To: GIT
When my boss has something to show me and I have to update, for some reason I
am always in the middle of doing something else, and git pull command refuses
to work in such a case.
I wrote this little script to save the changes I made, perform the update,
and then come back to where I was, but on top of the updated commit.
Here is how to use my script:
$ git save
$ git pull
$ git save restore
Signed-off-by: Nanako Shiraishi <nanako3@bluebottle.com>
diff --git a/git-save.sh b/git-save.sh
new file mode 100755
index 0000000..b45652e
--- /dev/null
+++ b/git-save.sh
@@ -0,0 +1,74 @@
+#! /bin/sh
+
+. git-sh-setup
+require_work_tree
+
+function save_work () {
+ save=$( (
+ i_tree=$(git-write-tree)
+
+ TMP=$GIT_DIR/.git-save-tmp
+ GIT_INDEX_FILE=$TMP
+ export GIT_INDEX_FILE
+
+ git-read-tree $i_tree
+ git-add -u
+ w_tree=$(git-write-tree)
+ rm $TMP
+ git-read-tree --prefix=base/ HEAD^{tree}
+ git-read-tree --prefix=indx/ $i_tree
+ git-read-tree --prefix=work/ $w_tree
+ git-write-tree
+ rm $TMP
+ ) )
+
+ head=$(git-log --abbrev-commit --pretty=oneline -n 1 HEAD)
+ if branch=$(git symbolic-ref -q HEAD); then
+ branch=${branch#refs/heads/}
+ else
+ branch='(no branch)'
+ fi &&
+ msg=$(printf 'WIP on %s "%s"' "$branch" "$head")
+
+ saved=$(printf '%s' "$msg" | git-commit-tree "$save")
+ git update-ref -m "$msg" refs/heads/saved $saved
+ printf 'Saved %s\n' "$msg" >&2
+}
+
+function list_save () {
+ git-log --abbrev-commit --pretty=oneline -g "$@" saved
+}
+
+function show_save () {
+ save=$(git rev-parse --verify --default saved "$1")
+ git diff-tree --stat $save:base $save:work
+}
+
+function restore_save () {
+ save=$(git rev-parse --verify --default saved "$1")
+ h_tree=$(git rev-parse --verify $save:base)
+ i_tree=$(git rev-parse --verify $save:indx)
+ w_tree=$(git rev-parse --verify $save:work)
+
+ git-merge-recursive $h_tree -- HEAD^{tree} $w_tree
+}
+
+if [ "$1" = list ]; then
+ shift
+ if [ "$#" = 0 ]; then
+ set x -n 10
+ shift
+ fi
+ list_save "$@"
+elif [ "$1" = show ]; then
+ shift
+ show_save "$@"
+elif [ "$1" = restore ]; then
+ shift
+ restore_save "$@"
+else
+ save_work
+ git reset --hard
+fi
+
+
--
しらいし ななこ http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Get a free email address with REAL anti-spam protection.
http://www.bluebottle.com
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 13:02 [PATCH] Add git-save script Nanako Shiraishi
@ 2007-06-23 15:05 ` Johannes Schindelin
2007-06-23 15:08 ` Bill Lear
` (2 more replies)
2007-06-23 20:15 ` Junio C Hamano
1 sibling, 3 replies; 12+ messages in thread
From: Johannes Schindelin @ 2007-06-23 15:05 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: GIT
Hi,
On Sat, 23 Jun 2007, Nanako Shiraishi wrote:
> Here is how to use my script:
>
> $ git save
> $ git pull
> $ git save restore
This use case has been discussed often, under the name "git-stash".
Ciao,
Dscho
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 15:05 ` Johannes Schindelin
@ 2007-06-23 15:08 ` Bill Lear
2007-06-23 15:13 ` Johannes Schindelin
2007-06-25 6:32 ` しらいしななこ
[not found] ` <200706250632.l5P6Wu6B028140@mi0.bluebottle.com>
2 siblings, 1 reply; 12+ messages in thread
From: Bill Lear @ 2007-06-23 15:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Nanako Shiraishi, GIT
On Saturday, June 23, 2007 at 16:05:22 (+0100) Johannes Schindelin writes:
>Hi,
>
>On Sat, 23 Jun 2007, Nanako Shiraishi wrote:
>
>> Here is how to use my script:
>>
>> $ git save
>> $ git pull
>> $ git save restore
>
>This use case has been discussed often, under the name "git-stash".
Was git-stash added as a patch yet? If not, do you think Nanako's
patch does not address the concerns that were raised in the
discussion you refer to?
Bill
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 15:08 ` Bill Lear
@ 2007-06-23 15:13 ` Johannes Schindelin
0 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2007-06-23 15:13 UTC (permalink / raw)
To: Bill Lear; +Cc: Nanako Shiraishi, GIT
Hi,
On Sat, 23 Jun 2007, Bill Lear wrote:
> On Saturday, June 23, 2007 at 16:05:22 (+0100) Johannes Schindelin writes:
> >Hi,
> >
> >On Sat, 23 Jun 2007, Nanako Shiraishi wrote:
> >
> >> Here is how to use my script:
> >>
> >> $ git save
> >> $ git pull
> >> $ git save restore
> >
> >This use case has been discussed often, under the name "git-stash".
>
> Was git-stash added as a patch yet? If not, do you think Nanako's
> patch does not address the concerns that were raised in the
> discussion you refer to?
Obviously, I prefer my approach of installing an alias.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 13:02 [PATCH] Add git-save script Nanako Shiraishi
2007-06-23 15:05 ` Johannes Schindelin
@ 2007-06-23 20:15 ` Junio C Hamano
2007-06-24 10:43 ` Nanako Shiraishi
2007-06-27 1:05 ` Junio C Hamano
1 sibling, 2 replies; 12+ messages in thread
From: Junio C Hamano @ 2007-06-23 20:15 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: GIT
Nanako Shiraishi <nanako3@bluebottle.com> writes:
> I wrote this little script to save the changes I made, perform the update,
> and then come back to where I was, but on top of the updated commit.
>
> Here is how to use my script:
>
> $ git save
> $ git pull
> $ git save restore
Heh, I earlier said that I'd start looking at the "stash", but
haven't done anything about it so far (I was busy with the day
job). More honestly, I was being lazy, and sometimes lazy are
rewarded, with a contribution from a new git hacker.
I think this is more or less in the direction I was planning to
head to; thanks for the patch.
Having said that, I have a few comments. Style, robustness,
and portability.
> diff --git a/git-save.sh b/git-save.sh
> new file mode 100755
> index 0000000..b45652e
> --- /dev/null
> +++ b/git-save.sh
> @@ -0,0 +1,74 @@
> +#! /bin/sh
Style. "#!/bin/sh" (no space after she-bang).
> +
> +. git-sh-setup
> +require_work_tree
> +
> +function save_work () {
No noiseword "function" for portability.
When you say "#!/bin/sh", you are writing for the family of
generic Bourne shells, not specifically for korn nor bash. For
example, dash is a fine POSIX shell, but does not grok function
noiseword. When in doubt, please stay away from things not in
POSIX (e.g. function, [[, ]], ${parameter//pattern/string/}).
> + save=$( (
> + i_tree=$(git-write-tree)
> +
> + TMP=$GIT_DIR/.git-save-tmp
> + GIT_INDEX_FILE=$TMP
> + export GIT_INDEX_FILE
> +
> + git-read-tree $i_tree
> + git-add -u
> + w_tree=$(git-write-tree)
> + rm $TMP
> + git-read-tree --prefix=base/ HEAD^{tree}
> + git-read-tree --prefix=indx/ $i_tree
> + git-read-tree --prefix=work/ $w_tree
> + git-write-tree
> + rm $TMP
> + ) )
Hmph.
I see many problems here, although the basic idea is sound and
"interesting". I especially like the use of three trees in a
single commit, without any parenthood.
- $GIT_DIR could contain shell metachararcters / whitespace, so
could $TMP as well. Always quote such variables, or you risk
a surprise from "rm".
- You probably would not want to create a new "save" if your
working tree and the index are clean. To test for the
condition, you can do something like:
git-diff --quiet --cached && git-diff --quiet
- I generally prefer cleaning temporary files with "rm -f", and
also set a trap upon exit, like this:
# near the beginning of the script
TMP="$GIT_DIR/.git-save-tmp-$$"
trap 'rm -f "$TMP"' 0
... and then much later ...
save=$( (
GIT_INDEX_FILE=$TMP
export GIT_INDEX_FILE
...
) )
- I wonder what happens if any of the commands in the above
sequence errors out. For example, I think you will get a
failure from the first git-write-tree if you are in the
middle of a merge. You would want to error out as soon as
you see a problem.
- Although you keep a separate tree for the index (before the
"git add -u" to grab the working tree changes) in the saved
data, it does not seem to be used. It _might_ make sense to
replace "git add -u" with "git add ." so that work/ tree
contains even untracked (but not ignored) files, and on the
restore side unstage the paths that appear in work/ but not
in indx/. I dunno.
> +
> + head=$(git-log --abbrev-commit --pretty=oneline -n 1 HEAD)
> + if branch=$(git symbolic-ref -q HEAD); then
> + branch=${branch#refs/heads/}
> + else
> + branch='(no branch)'
> + fi &&
Minor style. Please don't write "; then\n". Line-up "then",
"elif", "else", and "fi"; it is much easier to read that way.
> + msg=$(printf 'WIP on %s "%s"' "$branch" "$head")
> +
> + saved=$(printf '%s' "$msg" | git-commit-tree "$save")
> + git update-ref -m "$msg" refs/heads/saved $saved
> + printf 'Saved %s\n' "$msg" >&2
> +}
> +
> +function list_save () {
> + git-log --abbrev-commit --pretty=oneline -g "$@" saved
> +}
I really like the way reflog is used for this. Your saves do
not necessarily tied to a single branch (so there are no
parent-child relation between each save entry), but you can
still identify what each of the save is about because your log
message has branch name in it.
The sha1 at the beginning of oneline output, although they are
abbreviated, seem not very useful in this context, by the way.
I presume your intended use case is to view "git save list"
output, and say "git save restore saved@{2}" to apply the one
third from the top. For that use, the commit object name is
irrelevant. Maybe sed it out like this to make it even
prettier?
git-log --pretty=oneline -g "$@" saved
sed -e 's/^[0-9a-f]* //'
> +function show_save () {
> + save=$(git rev-parse --verify --default saved "$1")
> + git diff-tree --stat $save:base $save:work
> +}
Nice, but after trying this myself a bit, I seriously wished for
"git save show -p", so I did it myself, like this:
show_save () {
flags=$(git rev-parse --no-revs --flags "$@")
if test -z "$flags"
then
flags=--stat
fi
save=$(git rev-parse --revs-only --no-flags --default saved "$@")
git diff-tree $flags $save:base $save:work
}
It's a dense (and ancient style) plumbing code so needs a bit of
explanation:
- The first git-rev-parse looks at "$@", discards everything
that are not options and discards object names. So 'git save
show -p some' will give you "-p" in flags.
- The second one discards flags, and grabs 'some' out of '-p
some', or defaults to "saved".
> +function restore_save () {
> + save=$(git rev-parse --verify --default saved "$1")
> + h_tree=$(git rev-parse --verify $save:base)
> + i_tree=$(git rev-parse --verify $save:indx)
> + w_tree=$(git rev-parse --verify $save:work)
> +
> + git-merge-recursive $h_tree -- HEAD^{tree} $w_tree
> +}
The same "robustness" comments for the save_work function apply
here. You probably do not want to restore on a dirty tree; the
intended use case is "stash away, pull, then restore", so I
think it is Ok to assume that you will only be restoring on a
clean state (and it would make the implementation simpler).
The three-way merge is done correctly here, and I would imagine
the users would feel the UI quite natural _if_ this merge
conflicts. "git diff" would show only the conflicted paths
(because the updates coming from the old working tree is placed
in the index for cleanly merged paths), editing a conflicted
file and "git add $that_path" would resolve. That's exactly the
same workflow for a conflicted merge.
However, I think it is a bit counterintuitive to update the
working tree change to the index if the merge did not conflict.
It might be better to run an equivalent of "git reset", so that
after "git save restore", the user can say "git diff" (not "git
diff HEAD") to view the local changes. Perhaps...
if git-merge-recursive ...
then
# Cleanly merged..
added="$TMP" &&
git-diff --cached --name-only --diff-filter=A >"$added" &&
git-read-tree HEAD &&
git-update-index --add --stdin <"$added" ||
die "Cannot unstage local modification"
git-update-index --refresh
fi
The --diff-filter dance is so that we do not "forget" newly
added files when we do "read-tree HEAD".
> +
> +if [ "$1" = list ]; then
> + shift
> + if [ "$#" = 0 ]; then
> + set x -n 10
> + shift
> + fi
> + list_save "$@"
> +elif [ "$1" = show ]; then
> + shift
> + show_save "$@"
> +elif [ "$1" = restore ]; then
> + shift
> + restore_save "$@"
> +else
> + save_work
> + git reset --hard
I am not absolutely sure if "git reset --hard" belongs here.
You can certainly type one less command in your example sequence
("stash; pull; restore"). But I suspect there may be a case
that would be more useful if "git save" did not do the reset
itself. I dunno....
> +fi
> +
> +
Style. This is a perfect place to use "case/esac".
case "$1" in
list)
do this thing
;;
...
*)
do default thing
;;
esac
Much easier to read, isn't it?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 20:15 ` Junio C Hamano
@ 2007-06-24 10:43 ` Nanako Shiraishi
2007-06-24 19:45 ` Junio C Hamano
2007-06-27 1:05 ` Junio C Hamano
1 sibling, 1 reply; 12+ messages in thread
From: Nanako Shiraishi @ 2007-06-24 10:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT
Junio C Hamano <gitster@pobox.com> writes:
> Nanako Shiraishi <nanako3@bluebottle.com> writes:
>
>> +function save_work () {
>
> No noiseword "function" for portability.
>
> When you say "#!/bin/sh", you are writing for the family of
> generic Bourne shells, not specifically for korn nor bash. For
> example, dash is a fine POSIX shell, but does not grok function
> noiseword. When in doubt, please stay away from things not in
> POSIX (e.g. function, [[, ]], ${parameter//pattern/string/}).
Is there a good reference you can point me?
> - $GIT_DIR could contain shell metachararcters / whitespace, so
> could $TMP as well. Always quote such variables, or you risk
> a surprise from "rm".
>
> - You probably would not want to create a new "save" if your
> working tree and the index are clean. To test for the
> condition, you can do something like:
>
> git-diff --quiet --cached && git-diff --quiet
I did not think of these problems, but I understand now.
> - Although you keep a separate tree for the index (before the
> "git add -u" to grab the working tree changes) in the saved
> data, it does not seem to be used. It _might_ make sense to
> replace "git add -u" with "git add ." so that work/ tree
> contains even untracked (but not ignored) files, and on the
> restore side unstage the paths that appear in work/ but not
> in indx/. I dunno.
At first I wanted to do git-add . instead of git-add -u, but then I
became worried that will add files that are not interesting such as
temporary files.
>> +
>> + head=$(git-log --abbrev-commit --pretty=oneline -n 1 HEAD)
>> + if branch=$(git symbolic-ref -q HEAD); then
>> + branch=${branch#refs/heads/}
>> + else
>> + branch='(no branch)'
>> + fi &&
>
> Minor style. Please don't write "; then\n". Line-up "then",
> "elif", "else", and "fi"; it is much easier to read that way.
I see.
> Nice, but after trying this myself a bit, I seriously wished for
> "git save show -p", so I did it myself, like this:
>
> show_save () {
> flags=$(git rev-parse --no-revs --flags "$@")
> if test -z "$flags"
> then
> flags=--stat
> fi
> save=$(git rev-parse --revs-only --no-flags --default saved "$@")
> git diff-tree $flags $save:base $save:work
> }
>
> It's a dense (and ancient style) plumbing code so needs a bit of
> explanation:
>
> - The first git-rev-parse looks at "$@", discards everything
> that are not options and discards object names. So 'git save
> show -p some' will give you "-p" in flags.
>
> - The second one discards flags, and grabs 'some' out of '-p
> some', or defaults to "saved".
I did not know about these commands. I will study the manual pages
and I will re-submit my patch after adding these.
--
しらいし ななこ http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Get a free email account with anti spam protection.
http://www.bluebottle.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-24 10:43 ` Nanako Shiraishi
@ 2007-06-24 19:45 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2007-06-24 19:45 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: GIT
Nanako Shiraishi <nanako3@bluebottle.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
> ...
>> When you say "#!/bin/sh", you are writing for the family of
>> generic Bourne shells, not specifically for korn nor bash. For
>> example, dash is a fine POSIX shell, but does not grok function
>> noiseword. When in doubt, please stay away from things not in
>> POSIX (e.g. function, [[, ]], ${parameter//pattern/string/}).
>
> Is there a good reference you can point me?
I usually look at this:
http://www.opengroup.org/onlinepubs/000095399/
and click on "Shell and Utilities volume (XCU)", and compare it
with "man bash".
> At first I wanted to do git-add . instead of git-add -u, but then I
> became worried that will add files that are not interesting such as
> temporary files.
As long as .gitignore is set up properly to ignore such
generated files, that shouldn't be a problem.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 15:05 ` Johannes Schindelin
2007-06-23 15:08 ` Bill Lear
@ 2007-06-25 6:32 ` しらいしななこ
2007-06-25 7:52 ` Junio C Hamano
[not found] ` <200706250632.l5P6Wu6B028140@mi0.bluebottle.com>
2 siblings, 1 reply; 12+ messages in thread
From: しらいしななこ @ 2007-06-25 6:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: GIT
Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Sat, 23 Jun 2007, Nanako Shiraishi wrote:
>
> > Here is how to use my script:
> >
> > $ git save
> > $ git pull
> > $ git save restore
>
> This use case has been discussed often, under the name "git-stash".
>
> Ciao,
> Dscho
Thank you for your comments. Do you suggest I rename the script to git-stash and re-submit after fixing according to Junio's comments?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Finally - A spam blocker that actually works.
http://www.bluebottle.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
[not found] ` <200706250632.l5P6Wu6B028140@mi0.bluebottle.com>
@ 2007-06-25 7:45 ` Johannes Schindelin
0 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin @ 2007-06-25 7:45 UTC (permalink / raw)
To: しらいしななこ; +Cc: GIT
[-- Attachment #1: Type: TEXT/PLAIN, Size: 564 bytes --]
Hi,
On Mon, 25 Jun 2007, ã~A~Wã~B~Iã~A~Dã~A~Wã~Aªã~Aªã~A~S wrote:
> Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>
> > On Sat, 23 Jun 2007, Nanako Shiraishi wrote:
> >
> > > Here is how to use my script:
> > >
> > > $ git save
> > > $ git pull
> > > $ git save restore
> >
> > This use case has been discussed often, under the name "git-stash".
> >
> > Ciao,
> > Dscho
>
> Thank you for your comments. Do you suggest I rename the script to
> git-stash and re-submit after fixing according to Junio's comments?
Yes.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-25 6:32 ` しらいしななこ
@ 2007-06-25 7:52 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2007-06-25 7:52 UTC (permalink / raw)
To: しらいしななこ
Cc: Johannes Schindelin, GIT
しらいしななこ <nanako3@bluebottle.com> writes:
> Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>
>> On Sat, 23 Jun 2007, Nanako Shiraishi wrote:
>>
>> > Here is how to use my script:
>> >
>> > $ git save
>> > $ git pull
>> > $ git save restore
>>
>> This use case has been discussed often, under the name "git-stash".
>
> Thank you for your comments. Do you suggest I rename the
> script to git-stash and re-submit after fixing according to
> Junio's comments?
Side note: get into the habit of trimming the quote to
remove "Hi," and signatures, please.
Because many people on the list (not just me, as I was not
initially involved in the "wish: pulling into dirty tree"
discussion myself) already used the word "stash" for the concept
of the operation, I think it would give continuity to our
discussion if you used that word.
I have to also say that the "restore" subcommand had a certain
"Huh?" factor when I first saw the above "git save restore"
command line. I would have said "git stash" to save away, and
"git stash apply" to propagate the changes to updated HEAD.
That would make the set of subcommands to:
$ git stash -- to save away
$ git stash list -- to get the list
$ git stash show [$it] -- to view a single stash
$ git stash apply [$it] -- to apply changes from a stash
Right now you seem to use reflog so that "list" gives saved@{$n}
and the user is expected to pick from it and say something like
"git stash show saved@{2}", but I suspect that there is some
room for UI improvements.
* Is comandeering a branch name "saved" for the purpose of the
stash command a right thing to do?
Perhaps we would want to use refs/stash/ hierarchy;
* Is it more convenient to have a single stash that holds
changes you make anywhere, or is it better to have one stash
per branch?
Unlike StGIT and guilt, that are systems to manage patches
for longer term, I think the concept of stash is more geared
towards quickly stashing away local changes while you have to
'get distracted", as you described in your commit log
message. So in that sense, I think anything elaborate like
one stash per branch is a mistake, and a single stash that is
quickly aged and pruned automatically when reflog expires, as
you implemented, is the right approach. Taken together with
the previous point, I would actually suggest refs/stash as
the refname.
* If we settle on the design of having a single stash per
repository, the name of whatever the ref we use to implement
the stash should not have to be spelled out by the end user
(e.g. saved@{2} should not be necessary---the user should be
able to say "the stash marked with letter '2' in 'git stash
list' output).
So perhaps as a UI improvement, "git stash list" should show
them just numbered (strip away saved@{$N} part down to just
"$N:" or something), and "git stash show" and "git stash apply"
should take that number.
I think "git stash" is usable standalone, but in the particular
use case you mentioned, I was planning to follow Linus's
suggestion of stashing and unstashing automatically inside "git
pull" (most likely in "git merge", which is the underlying
command to do the actual merging part), when the pull results in
a fast-forward situation. But that would be a separate change
that uses "git stash" command.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-23 20:15 ` Junio C Hamano
2007-06-24 10:43 ` Nanako Shiraishi
@ 2007-06-27 1:05 ` Junio C Hamano
2007-06-27 9:38 ` しらいしななこ
1 sibling, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2007-06-27 1:05 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: GIT
Junio C Hamano <gitster@pobox.com> writes:
> Nanako Shiraishi <nanako3@bluebottle.com> writes:
> ...
>> + save=$( (
>> + i_tree=$(git-write-tree)
>> +
>> + TMP=$GIT_DIR/.git-save-tmp
>> + GIT_INDEX_FILE=$TMP
>> + export GIT_INDEX_FILE
>> +
>> + git-read-tree $i_tree
>> + git-add -u
>> + w_tree=$(git-write-tree)
>> + rm $TMP
>> + git-read-tree --prefix=base/ HEAD^{tree}
>> + git-read-tree --prefix=indx/ $i_tree
>> + git-read-tree --prefix=work/ $w_tree
>> + git-write-tree
>> + rm $TMP
>> + ) )
> ...
> - Although you keep a separate tree for the index (before the
> "git add -u" to grab the working tree changes) in the saved
> data, it does not seem to be used. It _might_ make sense to
> replace "git add -u" with "git add ." so that work/ tree
> contains even untracked (but not ignored) files, and on the
> restore side unstage the paths that appear in work/ but not
> in indx/. I dunno.
I would like to take the last part of the sentence back.
I do not think "git add ." makes much sense. The only case that
"add ." vs "add -u" may make a difference is the files that are
so new to your working tree that you haven't even added them to
the index. But they would not be in your current HEAD, and they
would not have been added in the commit you are pulling, and if
that is not the case, git-merge would complain and bail out as
usual anyway. I replied to your other message with something
about ".gitignore", but not everybody uses "ignore" mechanism
effectively, and "git add ." indeed would end up sucking all the
irrelevent cruft to w_tree for them, only to be extracted back
(because HEAD, i_tree and whichever commit you will later be
applying the stash to are not likely to have them). Maybe we
could argue that those people with nonoptimal "ignore" list
deserve it, but I do not think it is worth the potential added
aggravation.
By the way, I initially liked the clever use of a single commit
that records the three trees in it, but now I am beginning to
really hate it. I am suspecting that the primary reason you did
it that way was to reduce the number of commit objects used for
this purpose. I personally do not think it is something we
would want to optimize for.
I think it would make much more sense to represent a stash like
this:
.------o commit to represent index state
/ \
---o---o----------o
HEAD commit to represent worktree state
That is, "index" and "worktree" state are represented as one
commit each, both are direct child of the HEAD, with an added
twist of the latter being also a child of the former.
This has an interesting side effect that I can do this:
$ git show stash
to view what was in HEAD, index and working tree at the same
time (you can use "git stash show" to view the changes between
HEAD and worktree as before).
Being able to view this "evil merge" (this is a deliberately
evil merge, as you typically have huge differences between the
index and worktree when you need to stash) may not be that
interesting in the real life, but I think this is a good
demonstration of the reason why it is better to use more
straightforward ancestry structure to represent the stash. By
not using a custom tree structure that is only understood by
"git stash", the user can use existing tools that operate on
normal ancestry chains.
>> ...
>> + save_work
>> + git reset --hard
>
> I am not absolutely sure if "git reset --hard" belongs here.
> You can certainly type one less command in your example sequence
> ("stash; pull; restore"). But I suspect there may be a case
> that would be more useful if "git save" did not do the reset
> itself. I dunno....
I now think "git reset --hard" here is fine.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Add git-save script
2007-06-27 1:05 ` Junio C Hamano
@ 2007-06-27 9:38 ` しらいしななこ
0 siblings, 0 replies; 12+ messages in thread
From: しらいしななこ @ 2007-06-27 9:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT
Quoting Junio C Hamano <gitster@pobox.com>:
> I think it would make much more sense to represent a stash like
> this:
>
> .------o commit to represent index state
> / \
> ---o---o----------o
> HEAD commit to represent worktree state
>
> That is, "index" and "worktree" state are represented as one
> commit each, both are direct child of the HEAD, with an added
> twist of the latter being also a child of the former.
I do not know if I understand you correctly.
Do you mean that I should create a stash this way?
i_tree=$(git-write-tree)
i_commit=$(echo index | git-commit-tree $i_tree -p HEAD)
w_tree=$( what I did to create w_tree in my previous patch )
w_commit=$(echo $msg | git-commit-tree $w_tree -p HEAD -p $i_commit)
and when unstashing the stash, I should:
git-merge-recursive $stash^^{tree} -- $stash^^{tree} $stash^{tree}
I think I can make it work, but if that is not what you meant, please let me know.
> I am not absolutely sure if "git reset --hard" belongs here.
> > You can certainly type one less command in your example sequence
> > ("stash; pull; restore"). But I suspect there may be a case
> > that would be more useful if "git save" did not do the reset
> > itself. I dunno....
>
> I now think "git reset --hard" here is fine.
I see.
I will try to update and resend my patch this weekend.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
----------------------------------------------------------------------
Get a free email address with REAL anti-spam protection.
http://www.bluebottle.com
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2007-06-27 9:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-23 13:02 [PATCH] Add git-save script Nanako Shiraishi
2007-06-23 15:05 ` Johannes Schindelin
2007-06-23 15:08 ` Bill Lear
2007-06-23 15:13 ` Johannes Schindelin
2007-06-25 6:32 ` しらいしななこ
2007-06-25 7:52 ` Junio C Hamano
[not found] ` <200706250632.l5P6Wu6B028140@mi0.bluebottle.com>
2007-06-25 7:45 ` Johannes Schindelin
2007-06-23 20:15 ` Junio C Hamano
2007-06-24 10:43 ` Nanako Shiraishi
2007-06-24 19:45 ` Junio C Hamano
2007-06-27 1:05 ` Junio C Hamano
2007-06-27 9:38 ` しらいしななこ
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).