lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] PATCH: Submodule changes


From: Greg Chicares
Subject: Re: [lmi] PATCH: Submodule changes
Date: Fri, 9 Oct 2020 17:14:45 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0

On 2020-10-09 16:40, Vadim Zeitlin wrote:
> On Fri, 9 Oct 2020 16:05:32 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> On 2020-10-09 15:30, Vadim Zeitlin wrote:
> GC> > On Fri, 9 Oct 2020 15:12:03 +0000 Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
> GC> [...]
> GC> > GC> $ git --no-pager grep _sha install_wx* 
> GC> > GC> install_wx.sh:vendor=${LMI_TRIPLET}-$gcc_version-$wx_commit_sha
> GC> > GC> 
> install_wxpdfdoc.sh:build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"
> GC> > GC> 
> GC> > GC> ...still be used? AFAICT without actually rebuilding, those 
> variables
> GC> > GC> ought to be empty now, right?
> GC> > 
> GC> >  Oops, you're absolutely right and I just haven't noticed this. AFAICS
> GC> > there are just two solutions:
> GC> > 
> GC> > 1. Remove the SHA suffixes from the vendor string.
> GC> > 2. Set these suffixes to the actual commits used in the submodule.
> GC> > 
> GC> >  I guess you prefer the solution (2), so I'm going to do this, but how
> GC> > should I do it: wait until you push your changes and then make another 
> PR
> GC> > or update the existing PR with this change, so that you could 
> cherry-pick
> GC> > the new commit(s?)? What would you prefer?
> GC> 
> GC> Let's defer those questions, and plan to address them later,
> 
>  FWIW I think this should be addressed soon, as we probably don't want to
> start using wxWidgets DLLs with the names ending in "-" -- they work (we
> did test this), but look very unusual.

Let me make some suggestions and ask whether you disagree:

| install_wx.sh:vendor=${LMI_TRIPLET}-$gcc_version-$wx_commit_sha

It does seem useful to keep the names of compiled wx libraries
unique: that way, if we apply some minor change and rebuild them,
the result has a different name than the last version we released,
and we can see that immediately, just by listing the names (and
piping the list into 'less -S' because they're so terribly long).

I think I originally preferred to use the whole 40-byte SHA1
because we had previously used a 32-bit MD5SUM, and it was only
eight more bytes. Now that I understand better, I think it would
be more reasonable to use whatever "--abbrev-commit" SHA1 git uses
by default in the wx repository. (And if that grows from, say,
14 to 15 bytes next year, then we can let the name grow by one byte).

If you agree, would you please tell me exactly what to do? I could
spend an hour trying to figure out how to find the abbreviated SHA1
of the version currently checked out in a submodule, and still get
it wrong, so it's better for me to ask. But there's no need for you
to go to all the trouble of making a new PR for what I imagine is
a rather trivial change.

| 
install_wxpdfdoc.sh:build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"

This is just an arbitrary name for a build directory. In wxWidgets
and wxPdfDoc respectively, it's now:

  build_dir="$exec_prefix/wx-ad_hoc/lmi-$LMI_COMPILER-$gcc_version"
  build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"

Should we change the second one to be consistent with the first? Thus:

-  build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"
+  build_dir="$exec_prefix/wxpdfdoc-ad_hoc/lmi-$LMI_COMPILER-$gcc_version"

If you agree, I can just do that myself.

> GC> There are some 'shellcheck' complaints,

I think I know by now how to fix all of them (remembering not to quote
strings of command-line options where we really do want word-splitting
to take place). I'll probably do that first, once we agree on the
matters discussed above, which are a prerequisite because, e.g.:

In /opt/lmi/src/lmi/install_wx.sh line 59:
vendor=${LMI_TRIPLET}-$gcc_version-$wx_commit_sha
                                   ^------------^ SC2154: wx_commit_sha is 
referenced but not assigned.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]