Re: [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows

2013/1/26 Jeff King [off-list ref]:

On Fri, Jan 25, 2013 at 06:44:13PM +0400, Alexey Shumkin wrote:

quoted

 test_web_browse () {
-     # browser=$1 url=$2
+     # browser=$1 url=$2 sleep_timeout=$3
+     sleep_timeout="$3"
      git web--browse --browser="$1" "$2" >actual &&
+     # if $3 is set
+     # as far as Firefox is run in background (it is run with &)
+     # we trying to avoid race condition
+     # by waiting for "$sleep_timeout" seconds of timeout for
'fake_browser_ran' file appearance
+     (test -z "$sleep_timeout" || (
+         for timeout in $(seq 1 $sleep_timeout); do
+                     test -f fake_browser_ran && break
+                     sleep 1
+             done
+             test $timeout -ne $sleep_timeout
+             )
+     ) &&
      tr -d '\015' <actual >text &&

Gross, but I don't really see another way to handle the asynchronous
nature of spawning background browsers.

Two things, though:

  1. Should test_web_browse just delete fake_browser_ran for us? Then
     later tests do not have to remember to do so.

Yep, you're right

  2. Seeing fake_browser_ran appeared, we know that the script has
     started.  But there is still a race condition in which it may not
     have written anything to "actual" yet.

Definitely right

In this implementation:

quoted

+     cat >"fake browser" <<-\EOF &&
+     #!/bin/sh
+
+     : > fake_browser_ran
+     if test "$1" = "-version"; then
+             echo Fake Firefox browser version 1.2.3
+     else
+             # Firefox (in contrast to w3m) is run in background (with
&)
+             # so redirect output to "actual"
+             echo fake: "$@" > actual
+     fi
+     EOF

There is a period where fake_browser_ran exists, but nothing is in
actual. You can solve it by setting fake_browser_ran at the end rather
than the beginning.

Or you can drop fake_browser_ran entirely, and just atomically move
actual into place, like:

  echo "fake: $*" >actual.tmp
  mv actual.tmp actual

and then tes-t_web_browse can just spin waiting for "actual" to appear.

Not exactly, because, as I see, "actual" file is a result of redirection of

git web--browse --browser="$1" "$2" >actual &&

command

-Peff