* [TopGit PATCHv2] provide global temporary directory
@ 2010-10-09 21:41 Bert Wesarg
2010-10-09 21:53 ` Uwe Kleine-König
0 siblings, 1 reply; 3+ messages in thread
From: Bert Wesarg @ 2010-10-09 21:41 UTC (permalink / raw)
To: Uwe Kleine-Koenig; +Cc: git, pasky, martin f krafft, Bert Wesarg
The standard procedure 'tmp=mktemp; trap "rm $tmp" 0' was broken with the
introduction of the pager. Which overwrites the trap itself to close and
remove the pager fifo.
Now tg provides a temp playground and all other temp files should be created
inside this directory and only this directory will be removed with the exit
trap. setup_pager still overwrites the trap, but keeps the rm command from
the global temp directory. To simplify this the new function get_temp() is
provided.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
hooks/pre-commit.sh | 3 +--
tg-export.sh | 3 +--
tg-info.sh | 3 +--
tg-mail.sh | 4 +---
tg-patch.sh | 3 +--
tg-push.sh | 3 +--
tg-summary.sh | 3 +--
tg-update.sh | 3 +--
tg.sh | 27 +++++++++++++++++++--------
9 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/hooks/pre-commit.sh b/hooks/pre-commit.sh
index 4f2f16f..848a929 100644 hooks/pre-commit.sh
--- a/hooks/pre-commit.sh
+++ b/hooks/pre-commit.sh
@@ -95,9 +95,8 @@ BEGIN { in_hunk = 0; }
done
# check for repetitions of deps
-depdir="$(mktemp -t -d tg-depdir.XXXXXX)" ||
+depdir="$(get_temp -d tg-depdir)" ||
die "Can't check for multiple occurrences of deps"
-trap "rm -rf '$depdir'" 0
cat_file "(i):.topdeps" |
while read dep; do
[ ! -d "$depdir/$dep" ] ||
diff --git a/tg-export.sh b/tg-export.sh
index 6d82d55..921e933 100644 tg-export.sh
--- a/tg-export.sh
+++ b/tg-export.sh
@@ -57,8 +57,7 @@ if [ -z "$branches" ]; then
fi
-playground="$(mktemp -d -t tg-export.XXXXXX)"
-trap 'rm -rf "$playground"' EXIT
+playground="$(get_temp tg-export -d)"
## Collapse driver
diff --git a/tg-info.sh b/tg-info.sh
index 7d6a34c..10e257e 100644 tg-info.sh
--- a/tg-info.sh
+++ b/tg-info.sh
@@ -51,7 +51,7 @@ fi
git cat-file blob "$name:.topdeps" |
sed '1{ s/^/Depends: /; n; }; s/^/ /;'
-depcheck="$(mktemp -t tg-depcheck.XXXXXX)"
+depcheck="$(get_temp tg-depcheck)"
missing_deps=
needs_update "$name" >"$depcheck" || :
if [ -n "$missing_deps" ]; then
@@ -72,6 +72,5 @@ if [ -s "$depcheck" ]; then
else
echo "Up-to-date."
fi
-rm "$depcheck"
# vim:noet
diff --git a/tg-mail.sh b/tg-mail.sh
index 8167ade..dd4a95a 100644 tg-mail.sh
--- a/tg-mail.sh
+++ b/tg-mail.sh
@@ -34,7 +34,7 @@ if [ -n "$in_reply_to" ]; then
fi
-patchfile="$(mktemp -t tg-mail.XXXXXX)"
+patchfile="$(get_temp tg-mail)"
$tg patch "$name" >"$patchfile"
@@ -54,6 +54,4 @@ people=
# NOTE: git-send-email handles cc itself
eval git send-email $send_email_args "$people" "$patchfile"
-rm "$patchfile"
-
# vim:noet
diff --git a/tg-patch.sh b/tg-patch.sh
index 7bafdfe..68efcf0 100644 tg-patch.sh
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -51,7 +51,7 @@ echo
[ -n "$(git grep $diff_opts '^[-]--' ${diff_committed_only:+"$name"} -- ".topmsg")" ] || echo '---'
# Evil obnoxious hack to work around the lack of git diff --exclude
-git_is_stupid="$(mktemp -t tg-patch-changes.XXXXXX)"
+git_is_stupid="$(get_temp tg-patch-changes)"
git diff --name-only $diff_opts "$base_rev" ${diff_committed_only:+"$name"} -- |
fgrep -vx ".topdeps" |
fgrep -vx ".topmsg" >"$git_is_stupid" || : # fgrep likes to fail randomly?
@@ -61,7 +61,6 @@ if [ -s "$git_is_stupid" ]; then
else
echo "No changes."
fi
-rm "$git_is_stupid"
echo '-- '
echo "tg: ($base_rev..) $name (depends on: $(cat_file "$topic:.topdeps" | paste -s -d' '))"
diff --git a/tg-push.sh b/tg-push.sh
index 199d738..a928fba 100644 tg-push.sh
--- a/tg-push.sh
+++ b/tg-push.sh
@@ -45,8 +45,7 @@ for name in $branches; do
ref_exists "$name" || die "detached HEAD? Can't push $name"
done
-_listfile="$(mktemp -t tg-push-listfile.XXXXXX)"
-trap "rm -f \"$_listfile\"" 0
+_listfile="$(get_temp tg-push-listfile)"
push_branch()
{
diff --git a/tg-summary.sh b/tg-summary.sh
index af16888..612bd27 100644 tg-summary.sh
--- a/tg-summary.sh
+++ b/tg-summary.sh
@@ -55,10 +55,9 @@ EOT
fi
if [ -n "$sort" ]; then
- tsort_input=`mktemp`
+ tsort_input="$(get_temp tg-summary-sort)"
exec 4>$tsort_input
exec 5<$tsort_input
- rm $tsort_input
fi
## List branches
diff --git a/tg-update.sh b/tg-update.sh
index b256c7c..5162c52 100644 tg-update.sh
--- a/tg-update.sh
+++ b/tg-update.sh
@@ -27,7 +27,7 @@ base_rev="$(git rev-parse --short --verify "refs/top-bases/$name" 2>/dev/null)"
## First, take care of our base
-depcheck="$(mktemp -t tg-depcheck.XXXXXX)"
+depcheck="$(get_temp tg-depcheck)"
missing_deps=
needs_update "$name" >"$depcheck" || :
[ -z "$missing_deps" ] || die "some dependencies are missing: $missing_deps"
@@ -96,7 +96,6 @@ if [ -s "$depcheck" ]; then
else
info "The base is up-to-date."
fi
-rm "$depcheck"
# Home, sweet home...
# (We want to always switch back, in case we were on the base from failed
diff --git a/tg.sh b/tg.sh
index 8264a3b..3805eeb 100644 tg.sh
--- a/tg.sh
+++ b/tg.sh
@@ -150,7 +150,7 @@ recurse_deps()
_name="$1"; # no shift
_depchain="$*"
- _depsfile="$(mktemp -t tg-depsfile.XXXXXX)"
+ _depsfile="$(get_temp tg-depsfile)"
# If no_remotes is unset check also our base against remote base.
# Checking our head against remote head has to be done in the helper.
if test -z "$no_remotes" && has_remote "top-bases/$_name"; then
@@ -183,7 +183,6 @@ recurse_deps()
eval "$_cmd"
done <"$_depsfile"
missing_deps="${missing_deps# }"
- rm "$_depsfile"
return $_ret
}
@@ -334,19 +333,28 @@ setup_pager()
# now spawn pager
export LESS="${LESS:-FRSX}" # as in pager.c:pager_preexec()
- _pager_fifo_dir="$(mktemp -t -d tg-pager-fifo.XXXXXX)"
- _pager_fifo="$_pager_fifo_dir/0"
- mkfifo -m 600 "$_pager_fifo"
+ # setup_pager should be called only once per command
+ pager_fifo="$tg_tmp_dir/pager"
+ mkfifo -m 600 "$pager_fifo"
- "$TG_PAGER" < "$_pager_fifo" &
- exec > "$_pager_fifo" # dup2(pager_fifo.in, 1)
+ "$TG_PAGER" < "$pager_fifo" &
+ exec > "$pager_fifo" # dup2(pager_fifo.in, 1)
# this is needed so e.g. `git diff` will still colorize it's output if
# requested in ~/.gitconfig with color.diff=auto
export GIT_PAGER_IN_USE=1
# atexit(close(1); wait pager)
- trap "exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait" EXIT
+ # deliberately overwrites the global EXIT trap
+ trap "exec >&-; rm -rf \"$tg_tmp_dir\"; wait" EXIT
+}
+
+# get_temp NAME [-d]
+# creates a new temporary file (or directory with -d) in the global
+# temporary directory $tg_tmp_dir with pattern prefix NAME
+get_temp()
+{
+ mktemp ${2-} "$tg_tmp_dir/$1.XXXXXX"
}
## Startup
@@ -366,6 +374,9 @@ tg="tg"
# make sure merging the .top* files will always behave sanely
setup_ours
setup_hook "pre-commit"
+# create global temporary directories, inside GIT_DIR
+tg_tmp_dir="$(mktemp -d "$git_dir/tg-tmp.XXXXXX")"
+trap "rm -rf \"$tg_tmp_dir\"" EXIT
## Dispatch
--
1.7.1.1067.g5aeb7
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [TopGit PATCHv2] provide global temporary directory
2010-10-09 21:41 [TopGit PATCHv2] provide global temporary directory Bert Wesarg
@ 2010-10-09 21:53 ` Uwe Kleine-König
2010-10-10 7:47 ` Bert Wesarg
0 siblings, 1 reply; 3+ messages in thread
From: Uwe Kleine-König @ 2010-10-09 21:53 UTC (permalink / raw)
To: Bert Wesarg; +Cc: git, pasky, martin f krafft
On Sat, Oct 09, 2010 at 11:41:47PM +0200, Bert Wesarg wrote:
> The standard procedure 'tmp=mktemp; trap "rm $tmp" 0' was broken with the
> introduction of the pager. Which overwrites the trap itself to close and
> remove the pager fifo.
>
> Now tg provides a temp playground and all other temp files should be created
> inside this directory and only this directory will be removed with the exit
> trap. setup_pager still overwrites the trap, but keeps the rm command from
> the global temp directory. To simplify this the new function get_temp() is
> provided.
>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
> hooks/pre-commit.sh | 3 +--
> tg-export.sh | 3 +--
> tg-info.sh | 3 +--
> tg-mail.sh | 4 +---
> tg-patch.sh | 3 +--
> tg-push.sh | 3 +--
> tg-summary.sh | 3 +--
> tg-update.sh | 3 +--
> tg.sh | 27 +++++++++++++++++++--------
> 9 files changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/hooks/pre-commit.sh b/hooks/pre-commit.sh
> index 4f2f16f..848a929 100644 hooks/pre-commit.sh
> --- a/hooks/pre-commit.sh
> +++ b/hooks/pre-commit.sh
> @@ -95,9 +95,8 @@ BEGIN { in_hunk = 0; }
> done
>
> # check for repetitions of deps
> -depdir="$(mktemp -t -d tg-depdir.XXXXXX)" ||
> +depdir="$(get_temp -d tg-depdir)" ||
> die "Can't check for multiple occurrences of deps"
> -trap "rm -rf '$depdir'" 0
> cat_file "(i):.topdeps" |
> while read dep; do
> [ ! -d "$depdir/$dep" ] ||
> diff --git a/tg-export.sh b/tg-export.sh
> index 6d82d55..921e933 100644 tg-export.sh
> --- a/tg-export.sh
> +++ b/tg-export.sh
> @@ -57,8 +57,7 @@ if [ -z "$branches" ]; then
> fi
>
>
> -playground="$(mktemp -d -t tg-export.XXXXXX)"
> -trap 'rm -rf "$playground"' EXIT
> +playground="$(get_temp tg-export -d)"
>
>
> ## Collapse driver
> diff --git a/tg-info.sh b/tg-info.sh
> index 7d6a34c..10e257e 100644 tg-info.sh
> --- a/tg-info.sh
> +++ b/tg-info.sh
> @@ -51,7 +51,7 @@ fi
> git cat-file blob "$name:.topdeps" |
> sed '1{ s/^/Depends: /; n; }; s/^/ /;'
>
> -depcheck="$(mktemp -t tg-depcheck.XXXXXX)"
> +depcheck="$(get_temp tg-depcheck)"
> missing_deps=
> needs_update "$name" >"$depcheck" || :
> if [ -n "$missing_deps" ]; then
> @@ -72,6 +72,5 @@ if [ -s "$depcheck" ]; then
> else
> echo "Up-to-date."
> fi
> -rm "$depcheck"
>
> # vim:noet
> diff --git a/tg-mail.sh b/tg-mail.sh
> index 8167ade..dd4a95a 100644 tg-mail.sh
> --- a/tg-mail.sh
> +++ b/tg-mail.sh
> @@ -34,7 +34,7 @@ if [ -n "$in_reply_to" ]; then
> fi
>
>
> -patchfile="$(mktemp -t tg-mail.XXXXXX)"
> +patchfile="$(get_temp tg-mail)"
>
> $tg patch "$name" >"$patchfile"
>
> @@ -54,6 +54,4 @@ people=
> # NOTE: git-send-email handles cc itself
> eval git send-email $send_email_args "$people" "$patchfile"
>
> -rm "$patchfile"
> -
> # vim:noet
> diff --git a/tg-patch.sh b/tg-patch.sh
> index 7bafdfe..68efcf0 100644 tg-patch.sh
> --- a/tg-patch.sh
> +++ b/tg-patch.sh
> @@ -51,7 +51,7 @@ echo
> [ -n "$(git grep $diff_opts '^[-]--' ${diff_committed_only:+"$name"} -- ".topmsg")" ] || echo '---'
>
> # Evil obnoxious hack to work around the lack of git diff --exclude
> -git_is_stupid="$(mktemp -t tg-patch-changes.XXXXXX)"
> +git_is_stupid="$(get_temp tg-patch-changes)"
> git diff --name-only $diff_opts "$base_rev" ${diff_committed_only:+"$name"} -- |
> fgrep -vx ".topdeps" |
> fgrep -vx ".topmsg" >"$git_is_stupid" || : # fgrep likes to fail randomly?
> @@ -61,7 +61,6 @@ if [ -s "$git_is_stupid" ]; then
> else
> echo "No changes."
> fi
> -rm "$git_is_stupid"
>
> echo '-- '
> echo "tg: ($base_rev..) $name (depends on: $(cat_file "$topic:.topdeps" | paste -s -d' '))"
> diff --git a/tg-push.sh b/tg-push.sh
> index 199d738..a928fba 100644 tg-push.sh
> --- a/tg-push.sh
> +++ b/tg-push.sh
> @@ -45,8 +45,7 @@ for name in $branches; do
> ref_exists "$name" || die "detached HEAD? Can't push $name"
> done
>
> -_listfile="$(mktemp -t tg-push-listfile.XXXXXX)"
> -trap "rm -f \"$_listfile\"" 0
> +_listfile="$(get_temp tg-push-listfile)"
>
> push_branch()
> {
> diff --git a/tg-summary.sh b/tg-summary.sh
> index af16888..612bd27 100644 tg-summary.sh
> --- a/tg-summary.sh
> +++ b/tg-summary.sh
> @@ -55,10 +55,9 @@ EOT
> fi
>
> if [ -n "$sort" ]; then
> - tsort_input=`mktemp`
> + tsort_input="$(get_temp tg-summary-sort)"
> exec 4>$tsort_input
> exec 5<$tsort_input
> - rm $tsort_input
> fi
>
> ## List branches
> diff --git a/tg-update.sh b/tg-update.sh
> index b256c7c..5162c52 100644 tg-update.sh
> --- a/tg-update.sh
> +++ b/tg-update.sh
> @@ -27,7 +27,7 @@ base_rev="$(git rev-parse --short --verify "refs/top-bases/$name" 2>/dev/null)"
>
> ## First, take care of our base
>
> -depcheck="$(mktemp -t tg-depcheck.XXXXXX)"
> +depcheck="$(get_temp tg-depcheck)"
> missing_deps=
> needs_update "$name" >"$depcheck" || :
> [ -z "$missing_deps" ] || die "some dependencies are missing: $missing_deps"
> @@ -96,7 +96,6 @@ if [ -s "$depcheck" ]; then
> else
> info "The base is up-to-date."
> fi
> -rm "$depcheck"
>
> # Home, sweet home...
> # (We want to always switch back, in case we were on the base from failed
> diff --git a/tg.sh b/tg.sh
> index 8264a3b..3805eeb 100644 tg.sh
> --- a/tg.sh
> +++ b/tg.sh
> @@ -150,7 +150,7 @@ recurse_deps()
> _name="$1"; # no shift
> _depchain="$*"
>
> - _depsfile="$(mktemp -t tg-depsfile.XXXXXX)"
> + _depsfile="$(get_temp tg-depsfile)"
> # If no_remotes is unset check also our base against remote base.
> # Checking our head against remote head has to be done in the helper.
> if test -z "$no_remotes" && has_remote "top-bases/$_name"; then
> @@ -183,7 +183,6 @@ recurse_deps()
> eval "$_cmd"
> done <"$_depsfile"
> missing_deps="${missing_deps# }"
> - rm "$_depsfile"
> return $_ret
> }
>
> @@ -334,19 +333,28 @@ setup_pager()
> # now spawn pager
> export LESS="${LESS:-FRSX}" # as in pager.c:pager_preexec()
>
> - _pager_fifo_dir="$(mktemp -t -d tg-pager-fifo.XXXXXX)"
> - _pager_fifo="$_pager_fifo_dir/0"
> - mkfifo -m 600 "$_pager_fifo"
> + # setup_pager should be called only once per command
> + pager_fifo="$tg_tmp_dir/pager"
> + mkfifo -m 600 "$pager_fifo"
>
> - "$TG_PAGER" < "$_pager_fifo" &
> - exec > "$_pager_fifo" # dup2(pager_fifo.in, 1)
> + "$TG_PAGER" < "$pager_fifo" &
> + exec > "$pager_fifo" # dup2(pager_fifo.in, 1)
>
> # this is needed so e.g. `git diff` will still colorize it's output if
> # requested in ~/.gitconfig with color.diff=auto
> export GIT_PAGER_IN_USE=1
>
> # atexit(close(1); wait pager)
> - trap "exec >&-; rm \"$_pager_fifo\"; rmdir \"$_pager_fifo_dir\"; wait" EXIT
> + # deliberately overwrites the global EXIT trap
> + trap "exec >&-; rm -rf \"$tg_tmp_dir\"; wait" EXIT
> +}
> +
> +# get_temp NAME [-d]
I like your patch in general, but would prefer to have
# get_temp [-d] NAME
> +# creates a new temporary file (or directory with -d) in the global
> +# temporary directory $tg_tmp_dir with pattern prefix NAME
> +get_temp()
> +{
> + mktemp ${2-} "$tg_tmp_dir/$1.XXXXXX"
Does the - makes any difference? If yes, is it portable (enough)?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [TopGit PATCHv2] provide global temporary directory
2010-10-09 21:53 ` Uwe Kleine-König
@ 2010-10-10 7:47 ` Bert Wesarg
0 siblings, 0 replies; 3+ messages in thread
From: Bert Wesarg @ 2010-10-10 7:47 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: git, pasky, martin f krafft
2010/10/9 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> On Sat, Oct 09, 2010 at 11:41:47PM +0200, Bert Wesarg wrote:
>> +# get_temp NAME [-d]
> I like your patch in general, but would prefer to have
>
> # get_temp [-d] NAME
But that would complicate the argument evaluation, wouldn't it:
get_temp()
{
if [ $# -eq 2 ]; then
mktemp $1 "$tg_tmp_dir/$2.XXXXXX"
else
mktemp "$tg_tmp_dir/$1.XXXXXX"
fi
}
This and the functions with [-i | -w] are internal and won't be called
by the user, so we have full control over the calling convention and
should make it easy for us. And putting the optional argument at the
end is easy. I should probably remove the die call in cat_file,
because that would be an programmer error not an user error.
>
>> +# creates a new temporary file (or directory with -d) in the global
>> +# temporary directory $tg_tmp_dir with pattern prefix NAME
>> +get_temp()
>> +{
>> + mktemp ${2-} "$tg_tmp_dir/$1.XXXXXX"
> Does the - makes any difference? If yes, is it portable (enough)?
For me its an indicator, that it doesn't matter whether $2 was given
or not. Also you could run under set -u without error. But I don't do
this.
It's in POSIX [1].
Bert
[1] Statement given without interpretation.
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-10 7:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-09 21:41 [TopGit PATCHv2] provide global temporary directory Bert Wesarg
2010-10-09 21:53 ` Uwe Kleine-König
2010-10-10 7:47 ` Bert Wesarg
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).