Re: [lmi] [PATCH] Add script allowing to install any wxWidgets version from Git

[Greg Chicares]

On 2018-04-07 11:20, Vadim Zeitlin wrote:
> 
[...moving this command
> GC> -   [ -n "$skip_update" ] || git checkout "$wx_commit_sha"
...as was ultimately done in commit fdad3593f ...]
> 
>  To summarize, it's true that we don't have to do this test at all here any
> more (sorry for leaving it in, I didn't realize it became unnecessary after
> the refactoring I did), but the checkout command must be done in the "else"
> branch, i.e. only if HEAD != wx_commit_sha.

Why must git-checkout not be run if [ HEAD = wx_commit_sha ]?

When I cloned wx yesterday, HEAD happened to be at
  e38866d3a6 Merge branch 'lzma'
and indeed no git-checkout command was run. In that case, it would be
superfluous; but is it actually harmful? If I manually do this:

  $git checkout e38866d3a
  Note: checking out 'e38866d3a'.

  You are in 'detached HEAD' state.

then I'm at the same SHA1, though "detached"; but normally I won't
want HEAD, and I'll wind up with "detached HEAD" anyway.

If that is indeed harmless, then I'd like to move the code in this
"else" branch out, making it unconditional.

The less conditional logic, the better. BTW, I suspect there's a real
problem here: if the repository hasn't been cloned yet, then the
script clones it, implicitly checking out HEAD...and then it seems
not to check out the desired version. I think I should remove that
conditional too.

In the same vein, is there any strong reason not to unconditionalize
the code that handles submodules? I recorded 'md5sum .git/config', then
ran the commands for 'src/png' manually:

  $git config submodule.src/png.url https://github.com/wxWidgets/libpng.git
  $git submodule update --init src/png

  $time git config submodule.src/png.url https://github.com/wxWidgets/libpng.git
  0.00s user 0.00s system 0% cpu 0.004 total
  $time git submodule update --init src/png
  0.00s user 0.01s system 29% cpu 0.054 total

and the md5sum of .git/config didn't change.

Here's my proposed patch:

---------8<--------8<--------8<--------8<--------8<--------8<--------8<-------
diff --git a/install_wx.sh b/install_wx.sh
index 70f2119a..90f42da4 100755
--- a/install_wx.sh
+++ b/install_wx.sh
@@ -49,51 +49,40 @@ then
     [ -d $wx_dir_parent ] || mkdir -p $wx_dir_parent
     cd $wx_dir_parent
     git clone "$wx_git_url" ${wx_dir##*/}
-    cd $wx_dir
-else
-    cd $wx_dir
-    if [ $(git rev-parse HEAD) = "$wx_commit_sha" ]
-    then
-        # Don't bother updating anything if we already had the correct version
-        # of the tree.
-        skip_update=1
-    else
-        # Get the missing commit from the upstream repository if we don't have
-        # it yet.
-        if ! git rev-parse --quiet --verify "$wx_commit_sha^{commit}" 
>/dev/null
-        then
-            git fetch "$wx_git_url"
-        fi
-        git checkout "$wx_commit_sha"
-    fi
 fi
 
-if [ "$skip_update" != 1 ]
+cd $wx_dir
+
+# Fetch the desired commit from the upstream repository if it's missing.
+if ! git rev-parse --quiet --verify "$wx_commit_sha^{commit}" >/dev/null
 then
-    # Initialize all the not yet initialized submodules (except for the known
-    # exceptions, i.e. the submodules that we know that we won't need). This
-    # will be necessary after the initial clone, but may also need doing after
-    # updating an existing working tree if a new submodule is added upstream.
-    git submodule status | grep '^-' | cut -d' ' -f2 | while read -r subpath
-    do
-        case $subpath in
-            src/jpeg | src/tiff)
-                continue
-                ;;
-        esac
-
-        suburl=$(git config --file .gitmodules --get submodule.${subpath}.url)
-
-        # Configure the submodule to use URL relative to the one used for the
-        # super-repository itself: this doesn't change anything when using the
-        # canonical wxWidgets GitHub URL, but allows to download submodules
-        # from a local mirror when wxWidgets itself is being cloned from such
-        # a mirror, avoiding (slow and possibly unreliable) network access.
-        git config submodule.${subpath}.url ${wx_git_url%/*}/${suburl##*/}
-
-        git submodule update --init "$subpath"
-    done
+    git fetch "$wx_git_url"
 fi
+git checkout "$wx_commit_sha"
+
+# Initialize all the not yet initialized submodules (except for the known
+# exceptions, i.e. the submodules that we know that we won't need). This
+# will be necessary after the initial clone, but may also need doing after
+# updating an existing working tree if a new submodule is added upstream.
+git submodule status | grep '^-' | cut -d' ' -f2 | while read -r subpath
+do
+    case $subpath in
+        src/jpeg | src/tiff)
+            continue
+            ;;
+    esac
+
+    suburl=$(git config --file .gitmodules --get submodule.${subpath}.url)
+
+    # Configure the submodule to use URL relative to the one used for the
+    # super-repository itself: this doesn't change anything when using the
+    # canonical wxWidgets GitHub URL, but allows to download submodules
+    # from a local mirror when wxWidgets itself is being cloned from such
+    # a mirror, avoiding (slow and possibly unreliable) network access.
+    git config submodule.${subpath}.url ${wx_git_url%/*}/${suburl##*/}
+
+    git submodule update --init "$subpath"
+done
 
 [ "$wx_skip_clean" = 1 ] || git clean -dfx
 
--------->8-------->8-------->8-------->8-------->8-------->8-------->8-------