Bugfix 1395 develop comp script

[jprestop]

Pull Request Testing

• Describe testing already performed for these changes:
  

I tested the script on seneca:/d1/personal/jpresto/MET/test_comp using GCC version 8.3.0

I tested the script in several ways:

• full installation with all external libraries (needed to upgrade version of Freetype in tar_files to 2.7.0)
• with all libraries already installed (pointing to locations in environment config file)
• with and without compilation of HDF, HDFEOS, FREETYPE, and CAIRO.

I tested the script on cheyenne:/glade/p/ral/jntp/MET/MET_cross_platform_testing/met-10.1.0-beta2/gcc/
using GCC version 10.1.0

• Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
  

Feel free to run any additional tests you’d like (or not run any tests - I tried to be thorough and test everything). Please review the changes to the script, the documentation within the script, and updates to the documentation on the webpage (MET Downloads) which is currently marked as "Needs Review":
https://dtcenter.org/node/817/latest

• Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]

• Do these changes include sufficient testing updates? [Yes]

• Will this PR result in changes to the test suite? [No]
  
  If yes, describe the new output and/or changes to the existing output:
  

• Please complete this pull request review by 10/5/21 (before beta3 release if possible).
  

Pull Request Checklist

See the METplus Workflow for details.

• Complete the PR definition above.
• Ensure the PR title matches the feature or bugfix branch name.
• Define the PR metadata, as permissions allow.
  Select: Reviewer(s)
  Select: Organization level software support Project or Repository level development cycle Project
  Select: Milestone as the version that will include these changes
• After submitting the PR, select Linked issues with the original issue number.
• After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
• Close the linked issue and delete your feature or bugfix branch from GitHub.

[jprestop]

@JohnHalleyGotway I should mention that while this is a bugfix to fix the issue where it breaks when GNU versions lower than 10 are used I do not plan to do a branch off of main_v10.0 because folks will be grabbing the compile script from the website and not from the repo directly. Plus, the version of the compile script in main_v10.0 is the script I used to install version 10.0 on various machines, so I'd like to keep that as-is.

[JohnHalleyGotway]

@jprestop I browsed the changes to script and noticed some odd wrapping of lines. When I opened it in an editor on my Mac, I found lots of trailing whitespace on many lines. I ran the following commands to strip them out and then committed the result:

cat scripts/installation/compile_MET_all.sh | sed -r 's/ +$//g' > t
mv t scripts/installation/compile_MET_all.sh 

I see no need for change to the actual content of the script. But adding all those extra details to the top seems very helpful to users! However the formatting has more issues. Scrolling through the body, I see a mixture of tabs and spaces that result in inconsistent indentation... sometimes 2 spaces, sometimes 4, and sometimes with tabs. The MET code typically indents 3 spaces and in scripts like this I usually use 2 spaces. But just keeping it consistent throughout the file makes it easier to read.

I don't think it's worthwhile for me to actually out the script since you've already run it so many times in so many places. If you think that'd be helpful, I can, but I'll wait for you to tell me to do so.

Regarding the website (https://dtcenter.org/node/817/latest), two comments:

• Instead of listing many machines '(includes NCAR RAL machines and NCAR machine cheyenne, and NOAA machines theia, jet, and WCOSS)' could we be more generic, like '(includes widely used HPC's at NCAR and NOAA)'? For example, perhaps jet will be retired and 5 years later, we'll still have a reference to it on this page.
• I wish there were some way to remove version-specific instructions so we wouldn't need to update this page for each release. While "https://github.com/dtcenter/MET/releases/latest" does resolve to "v10.0.0" I don't think there's a way to get the "latest" tarfile. We could point them to the "latest" release page and tell them to click the button to download the tarfile. But having commands to copy/paste is pretty convenient. But whenever we made edits to websites, I'm always trying to minimize future maintenance because we usually don't do it very well!

I'm going to select the "request changes" option since I think getting the indentation consistent is worth doing.

[jprestop]

@JohnHalleyGotway Thanks for the review, for getting rid of the trailing whitespace, and letting me know the command that you used to do it. There was definitely a mix of tabs and varying spaces! I've been playing with my emacs options trying to fix that problem. Hopefully, I'll get that resolved. In any case, I fixed up this file appropriately, sticking with two spaces for consistency. To ensure that I didn't mess up the actual indentation of if statements and whatnot, I did a manual check with the old version to ensure the indentation had remained the same and I also tested again with the installation of all libraries to ensure they all compiled well still.

I don't think there is any need for you to test the script. I tried to be thorough in testing.

Regarding:

Instead of listing many machines '(includes NCAR RAL machines and NCAR machine cheyenne, and NOAA machines theia, jet, and WCOSS)' could we be more generic, like '(includes widely used HPC's at NCAR and NOAA)'? For example, perhaps jet will be retired and 5 years later, we'll still have a reference to it on this page.

Ha! I found an old reference to "theia" on the page, so I changed that to "hera" and decided to go generic with "includes widely used HPC's at NCAR and NOAA" in the place you suggested. So those changes have been made.

Regarding:

I wish there were some way to remove version-specific instructions so we wouldn't need to update this page for each release. While "https://github.com/dtcenter/MET/releases/latest" does resolve to "v10.0.0" I don't think there's a way to get the "latest" tarfile. We could point them to the "latest" release page and tell them to click the button to download the tarfile. But having commands to copy/paste is pretty convenient. But whenever we made edits to websites, I'm always trying to minimize future maintenance because we usually don't do it very well!

I agree and wasn't sure about that one. I went ahead and changed it from:

Download the latest MET release (e.g. met-10.0.0.20210510.tar.gz) from the Download page into the tar_files directory:
wget https://github.com/dtcenter/MET/releases/download/v10.0.0/met-10.0.0.20210510.tar.gz

To:

Download the latest MET release (e.g. met-10.0.0.tar.gz) from the Recommended download section above using a wget (or other) command and place it in the tar_files directory.

I know met-10.0.0.tar.gz is still listed, but it is only as an example. Please let me know what you think of that change. If it's good, I'll go ahead and publish it once this is merged.

[jprestop]

@JohnHalleyGotway

I see I didn't modify my comment above before submitting.

I changed the reference that included hera from:

includes NCAR RAL machines and NCAR machine cheyenne, and NOAA machines theia, jet, and WCOSS

to:

includes NCAR RAL machines and widely used HPC's at NCAR and NOAA

to get rid of references to all specific machines.

[JohnHalleyGotway]

Thanks for fixing up the indentation. Looks good to me now. I approve.