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

[Vadim Zeitlin]

On Sat, 7 Apr 2018 09:35:02 +0000 Greg Chicares <address@hidden> wrote:

GC> On 2018-04-06 22:42, Vadim Zeitlin wrote:
GC> > On Fri, 6 Apr 2018 13:31:31 +0000 Greg Chicares <address@hidden> wrote:
GC> > 
GC> > [...]
GC> > GC> > Should we do it like this?
GC> > GC> 
GC> > GC> If I've understood this correctly, then yes, please.
GC> > 
GC> >  I think https://github.com/vadz/lmi/pull/80 should do it, at least it
GC> > seems to work for me both when cloning from the GitHub wxWidgets 
repository
GC> > and when cloning from a local mirror.
GC> 
GC> Let me ask a couple questions about 'skip_update'.
GC> 
GC> (1) It occurs only on these three lines:
GC> 
GC> $grep skip_update install_wx.sh 
GC>         skip_update=1
GC>     [ -n "$skip_update" ] || git checkout "$wx_commit_sha"
GC> if [ "$skip_update" != 1 ]
GC> 
GC> so it can only be either null or 1, but the tests differ. Can I replace the
GC> last line as follows, just for uniformity, or am I missing some nuance?
GC> 
GC> -if [ "$skip_update" != 1 ]
GC> +if [ -n "$skip_update" ]
GC> 
GC> Oh, wait: '-n" doesn't test whether it's null: it tests whether it's 
nonnull.
GC> So '-z' would be the appropriate alternative there. But because I obviously
GC> can't make sense of '-n' without extreme effort, let me ask instead whether
GC> the middle line could be replaced thus:
GC> -   [ -n "$skip_update" ] || git checkout "$wx_commit_sha"
GC> +   [ "$skip_update" = 1 ] || git checkout "$wx_commit_sha"

 Yes, sure. Just to explain how did I end up with the original version: I
like using "-n", but I think that using both "-n" and "-z" may be
confusing, so I wanted to avoid it but OTOH using "! -n" when "-z" exists
didn't seem a good idea neither, so I used "$skip_update" != 1. But it's
true that using "$skip_update" = 1 or != 1 everywhere would be more
consistent.

GC> (2) Wouldn't the following be a pure refactoring (and simpler)?

 Sorry, not quite.

GC>     if [ $(git rev-parse HEAD) = "$wx_commit_sha" ]
GC>     then
GC> +       git checkout "$wx_commit_sha"

 This should go into the "else" branch: we only want to checkout this
commit if HEAD does _not_ already point to it. And this is what the current
test using "-n" does.

GC>         # Don't bother updating anything if we already had the correct 
version
GC>         # of the tree.
GC>         skip_update=1
GC>     else
GC>         # Get the missing commit from the upstream repository if we don't 
have
GC>         # it yet.
GC>         if ! git rev-parse --quiet --verify "$wx_commit_sha^{commit}" 
>/dev/null
GC>         then
GC>             git fetch "$wx_git_url"
GC>         fi
GC> [or move the '+' line here if I still don't understand 'test -n']

 Yes, the test using "-n" means "true if $skip_update is not empty", so
"git checkout" is executed only if this test is false.

GC>     fi
GC> 
GC> -   [ -n "$skip_update" ] || git checkout "$wx_commit_sha"

 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.


GC> I feel certain that I want a local mirror, and I imagine I want it to
GC> contain "real" bare repositories.

 Yes, I think it's a better idea in the long term, I just was trying to say
that you could test this quickly even without creating them right now.

GC> Let's take libpng as an example.
GC> - Do I mirror "git://git.code.sf.net/p/libpng/code"? Or does wx git
GC>   contain its own libpng git repository that I can mirror instead?

 It's the latter and the URL is in .gitmodules file (this is what it's
for!):

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

GC>   And if so, would you please tell me the command to do that?

 It's just this:

        $ git clone --bare https://github.com/wxWidgets/libpng.git

(again, you can speed things up by cloning your existing submodule or, at
least, using --reference "git clone" option to point to it, but this is
just an optimization and if you're not in a hurry, you can just run the
command above and let it take its time).

GC> - If I can mirror a bare libpng repository from wx, then does that
GC>   avoid the
GC>     
https://github.com/wxWidgets/wxWidgets/blob/master/docs/contributing/how-to-update-third-party-library.md
GC>     "We use a special hack for libpng"
GC>   issues, or do I have to implement that special hack myself?

 No, you don't have to do anything at all, this is only relevant when
updating wxWidgets to use a newer libpng version and not when simply using
wxWidgets.


GC> I ran './install_wx.sh' before PR #80, which executed this line:
GC>   git clone "$wx_git_url" ${wx_dir##*/}
GC> and I ran
GC>   wx_skip_clean=1 ./install_wx.sh
GC> after applying PR #80. Now the clone is up to date:
GC>   /opt/lmi/third_party/src/wxWidgets[0]$git rev-parse HEAD                  
         
GC>   41045df7ea5f93e4c07c1bd846d7127a372705bd
GC> and, if I understand the 'skip_update' logic correctly, there's
GC> nothing more that the script should do--the new PR #80 section
GC> is skipped. If that's right, then I should have all the submodules
GC> I need. But I seem to have none, because there's no '.gitmodules':
GC>   /opt/lmi/third_party/src/wxWidgets[0]$ls -a .git/       
GC>   .  ..  HEAD  branches  config  description  hooks  index  info  logs  
objects  packed-refs  refs

 It's not under .git, but in the repository itself, however...

GC> And I had hoped that maybe wxWidgets/src/png would contain its
GC> own '.git' subdirectory, but it doesn't. Is this merely because
GC> 41045df7ea didn't yet use submodules?

... you wouldn't find .gitmodules even in the repository root in this
commit because submodules hadn't been used then yet:

        % git show 41045df7ea:.gitmodules
        fatal: Path '.gitmodules' exists on disk, but not in '41045df7ea'.

 If you rerun the script now using a later commit, e.g.
wx_commit_sha=379a404f9804ac044df44f18907e192917fec8aa (this is v3.1.1),
it should initialize all the submodules. If you don't set up wx_git_url to
point to a local mirror, it will get them from GitHub, i.e., again, it will
take some time to do it -- but it should still work.

 Please let me know if you have any other questions/problems,
VZ