Jeff King <peff@xxxxxxxx> writes:
> On Wed, May 04, 2011 at 08:42:58AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@xxxxxxxx> writes:
>>
>> > ... but I worry about compatibility. We can audit our in-tree scripts,
>> > but git-sh-setup is part of the recommended API for external scripts,
>> > no?
>>
>> I am Ok with renaming the thing to "require_work_tree_exists", and all the
>> worry will go away.
>
> Yeah, that seems like a fine solution to me.
Ok, then here is a reroll.
-- >8 --
Somebody tried "git pull" from a random place completely outside the work
tree, while exporting GIT_DIR and GIT_WORK_TREE that are set to correct
places, e.g.
GIT_WORK_TREE=$HOME/git.git
GIT_DIR=$GIT_WORK_TREE/.git
export GIT_WORK_TREE GIT_DIR
cd /tmp
git pull
At the beginning of git-pull, we check "require-work-tree" and then
"cd-to-toplevel". I _think_ the original intention when I wrote the
command was "we MUST have a work tree, our $(cwd) might not be at the
top-level directory of it", and no stronger than that. That check is a
very sensible thing to do before doing cd-to-toplevel. We check that the
place we would want to go exists, and then go there.
But the implementation of require_work_tree we have today is quite
different. I don't have energy to dig the history, but currently it says:
test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
die "fatal: $0 cannot be used without a working tree."
Which is completely bogus. Even though we may happen to be just outside
of it right now, we may have a working tree that we can cd_to_toplevel
back to.
Add a function "require_work_tree_exists" that implements the check
this function originally intended (this is so that third-party scripts
that rely on the current behaviour do not have to get broken), and
update all the in-tree users to use it.
Some comments:
- The call in git-rebase--interactive.sh can actually be removed, I
think, as it is always called from git-rebase.sh that has done
cd_to_toplevel already.
- I am not sure if git-submodule.sh correctly works with the "as long as
there is a working tree somewhere, it is OK" semantics. It may have
to stay "require_work_tree" to ensure that the $cwd is within the
working tree.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
Documentation/git-sh-setup.txt | 11 ++++++++---
git-am.sh | 2 +-
git-bisect.sh | 2 +-
git-mergetool.sh | 2 +-
git-pull.sh | 2 +-
git-rebase--interactive.sh | 2 +-
git-rebase.sh | 2 +-
git-sh-setup.sh | 7 +++++++
git-stash.sh | 2 +-
git-submodule.sh | 2 +-
10 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 3da2413..1f02c4b 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -58,9 +58,14 @@ cd_to_toplevel::
runs chdir to the toplevel of the working tree.
require_work_tree::
- checks if the repository is a bare repository, and dies
- if so. Used by scripts that require working tree
- (e.g. `checkout`).
+ checks if the current directory is within the working tree
+ of the repository, and otherwise dies.
+
+require_work_tree_exists::
+ checks if the working tree associated with the repository
+ exists, and otherwise dies. Often done before calling
+ cd_to_toplevel, which is impossible to do if there is no
+ working tree.
get_author_ident_from_commit::
outputs code for use with eval to set the GIT_AUTHOR_NAME,
diff --git a/git-am.sh b/git-am.sh
index 6cdd591..430bbd5 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -39,7 +39,7 @@ rebasing* (internal use for git-rebase)"
. git-sh-setup
prefix=$(git rev-parse --show-prefix)
set_reflog_action am
-require_work_tree
+require_work_tree_exists
cd_to_toplevel
git var GIT_COMMITTER_IDENT >/dev/null ||
diff --git a/git-bisect.sh b/git-bisect.sh
index c21e33c..f46dda2 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -28,7 +28,7 @@ Please use "git help bisect" to get the full man page.'
OPTIONS_SPEC=
. git-sh-setup
-require_work_tree
+require_work_tree_exists
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 2f8dc44..8a74d00 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -14,7 +14,7 @@ OPTIONS_SPEC=
TOOL_MODE=merge
. git-sh-setup
. git-mergetool--lib
-require_work_tree
+require_work_tree_exists
# Returns true if the mode reflects a symlink
is_symlink () {
diff --git a/git-pull.sh b/git-pull.sh
index eb87f49..7500b58 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -10,7 +10,7 @@ SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
. git-sh-setup
set_reflog_action "pull $*"
-require_work_tree
+require_work_tree_exists
cd_to_toplevel
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5873ba4..051a4ad 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -34,7 +34,7 @@ autosquash move commits that begin with squash!/fixup! under -i
"
. git-sh-setup
-require_work_tree
+require_work_tree_exists
DOTEST="$GIT_DIR/rebase-merge"
diff --git a/git-rebase.sh b/git-rebase.sh
index cbb0ea9..60a405d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -31,7 +31,7 @@ SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
. git-sh-setup
set_reflog_action rebase
-require_work_tree
+require_work_tree_exists
cd_to_toplevel
LF='
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index aa16b83..94e26ed 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -140,6 +140,13 @@ cd_to_toplevel () {
}
}
+require_work_tree_exists () {
+ if test "z$(git rev-parse --is-bare-repository)" != zfalse
+ then
+ die "fatal: $0 cannot be used without a working tree."
+ fi
+}
+
require_work_tree () {
test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
die "fatal: $0 cannot be used without a working tree."
diff --git a/git-stash.sh b/git-stash.sh
index 7561b37..ce633f3 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -13,7 +13,7 @@ USAGE="list [<options>]
SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
. git-sh-setup
-require_work_tree
+require_work_tree_exists
cd_to_toplevel
TMP="$GIT_DIR/.git-stash.$$"
diff --git a/git-submodule.sh b/git-submodule.sh
index 8b90589..f2a541e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -15,7 +15,7 @@ USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <r
OPTIONS_SPEC=
. git-sh-setup
. git-parse-remote
-require_work_tree
+require_work_tree_exists
command=
branch=
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html