Re: [lmi] PATCH: Submodule changes

[Vadim Zeitlin]

On Fri, 9 Oct 2020 17:14:45 +0000 Greg Chicares <gchicares@sbcglobal.net> wrote:

GC> Let me make some suggestions and ask whether you disagree:
GC> 
GC> | install_wx.sh:vendor=${LMI_TRIPLET}-$gcc_version-$wx_commit_sha
GC> 
GC> It does seem useful to keep the names of compiled wx libraries
GC> unique: that way, if we apply some minor change and rebuild them,
GC> the result has a different name than the last version we released,
GC> and we can see that immediately, just by listing the names (and
GC> piping the list into 'less -S' because they're so terribly long).
GC> 
GC> I think I originally preferred to use the whole 40-byte SHA1
GC> because we had previously used a 32-bit MD5SUM, and it was only
GC> eight more bytes. Now that I understand better, I think it would
GC> be more reasonable to use whatever "--abbrev-commit" SHA1 git uses
GC> by default in the wx repository. (And if that grows from, say,
GC> 14 to 15 bytes next year, then we can let the name grow by one byte).

 Yes, this seems reasonable. I usually use `git describe --tags` output
which gives a better idea about the commit because it includes the latest
tag before it, which usually corresponds to some release, but this would be
redundant here because the DLL names already include the version anyhow.
And git describe output is a bit too long, e.g. it's v3.1.4-327-g3e88df56f5
for wx right now, which comes to 22 characters.

GC> If you agree, would you please tell me exactly what to do? I could
GC> spend an hour trying to figure out how to find the abbreviated SHA1
GC> of the version currently checked out in a submodule, and still get
GC> it wrong, so it's better for me to ask.

 AFAIK the way currently used in .github/workflows/ci.yml is the simplest
and hence the best one, we just need to use --short option to get the
abbreviated form. So the full command to get the current wx submodule SHA
would become

        $ git rev-parse --short HEAD:third_party/wx

("HEAD" here could be omitted, but IMO it looks more clear with it).

GC> | 
install_wxpdfdoc.sh:build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"
GC> 
GC> This is just an arbitrary name for a build directory. In wxWidgets
GC> and wxPdfDoc respectively, it's now:
GC> 
GC>   build_dir="$exec_prefix/wx-ad_hoc/lmi-$LMI_COMPILER-$gcc_version"
GC>   build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"
GC> 
GC> Should we change the second one to be consistent with the first?

 Consistency is always good, and I also don't think having the SHA in the
directory name is useful -- or, rather, I'd argue that it's harmful because
it doesn't allow you to reuse the same build directory if you make even a
tiny commit when you use wxpdfdoc_skip_clean=1.

GC> Thus:
GC> 
GC> -  build_dir="$exec_prefix/wxpdfdoc-ad_hoc/wxpdfdoc-$wxpdfdoc_commit_sha"
GC> +  build_dir="$exec_prefix/wxpdfdoc-ad_hoc/lmi-$LMI_COMPILER-$gcc_version"
GC> 
GC> If you agree, I can just do that myself.

 So yes, I definitely agree with this change, thanks in advance!
VZ

pgp7Jf0QDPEnp.pgp
Description: PGP signature