visit-dav / visit

VisIt - Visualization and Data Analysis for Mesh-based Scientific Data
https://visit.llnl.gov
BSD 3-Clause "New" or "Revised" License
438 stars 116 forks source link

Consistify OS name/version queries in build_visit #4494

Open cyrush opened 4 years ago

cyrush commented 4 years ago

Describe the bug

context: https://github.com/visit-dav/visit/pull/4492

we have multiple ways to probe macOS environment issues. I think in most cases we use MACOSX_DEPLOYMENT_TARGET, but in XDMF -- we use

/usr/bin/xcodebuild -version

MACOSX_DEPLOYMENT_TARGET is the version of macOS, i think thats easier to manage than the version of xcode.

We can investigate if MACOSX_DEPLOYMENT_TARGET is suitable try to reduce the patterns used in build_visit

Impact

Likelihood

Desktop

markcmiller86 commented 4 years ago

My recollection from the distant past is that down in the bowels of CMake, it too uses (used to use) MACOSX_DEPLOYMENT_TARGET (and friends) as well and also that there were cases, perhaps having to do with how or if Xcode was installed, that impact the existence and validity of this variable and then ultimately how CMake behaved.

griffin28 commented 4 years ago

Here's another way:

if [[ "$OPSYS" == "Darwin" ]]; then if [[ sw_vers -productVersion == 10.14.[0-9]* ]]; then apply_qt_5101_macos_mojave_patch if [[ $? != 0 ]] ; then return 1 fi fi
fi

markcmiller86 commented 4 years ago

Ok, so @griffin28 found a 3rd approach. And, from this ref there is a 4th approach system_profiler SPSoftwareDataType...

miller86% system_profiler SPSoftwareDataType
Software:

    System Software Overview:

      System Version: macOS 10.13.6 (17G11023)
      Kernel Version: Darwin 17.7.0
      Boot Volume: Macintosh HD
      Boot Mode: Normal
      Computer Name: scratlantis
      User Name: Miller, Mark C. (miller86)
      Secure Virtual Memory: Enabled
      System Integrity Protection: Enabled
      Time since boot: 2 days 10:30
markcmiller86 commented 4 years ago

And another...

miller86% defaults read loginwindow SystemVersionStampAsString
10.13.6
markcmiller86 commented 4 years ago

Inventory

bv_main defines value for MACOSX_DEPLOYMENT_TARGET by doing uname -r which returns a funky number like 17.7.1 and then maintaining an internal mapping between the first digit of that number to OSX/macOS minor version numbers. From a stackoverflow post

uname         OSX/macOS vers
----------------------------
17.x.x. macOS 10.13.x High Sierra
16.x.x  macOS 10.12.x Sierra
15.x.x  OS X  10.11.x El Capitan
14.x.x  OS X  10.10.x Yosemite
13.x.x  OS X  10.9.x  Mavericks
12.x.x  OS X  10.8.x  Mountain Lion
11.x.x  OS X  10.7.x  Lion
10.x.x  OS X  10.6.x  Snow Leopard
 9.x.x  OS X  10.5.x  Leopard
 8.x.x  OS X  10.4.x  Tiger
 7.x.x  OS X  10.3.x  Panther
 6.x.x  OS X  10.2.x  Jaguar
 5.x    OS X  10.1.x  Puma

The uname -r appears to be querying the Darwin version and from that we infer, by maintaining the mapping above in bv_main.sh the OSX/macOS version. [Side note: The above stackoverflow reference also includes details on obtaining OSX/macOS version at run time of an executing C application.]

In addition, we set OPSYS to Darwin when on a Mac and then all bv_xxx.sh files use tests like if [[ "$OPSYS" == "Darwin" ]]; then....

This all seems far too convoluted.

For a) consistency and b) portability, I propose...

markcmiller86 commented 4 years ago

Regarding the importance of MACOSX_DEPLOYMENT_TARGET, I found this authoritative reference which verifys the that this variable must be set explicitly when building a project outside of Xcode.

cyrush commented 4 years ago

Yes, but I think we do set MACOSX_DEPLOYMENT_TARGET?