[DO NOT MERGE] Unmerged changes in v0.10.0-RC branch

[rhornung67]

Summary

• This PR compares the unmerged changes in v0.10.0-RC branch to develop.
• It includes version updates and updated host-configs and CI updates for new versions of TPLs .

Please review this PR, but approve #1425 for merging.

[bmhan12]

It is certainly related. The errors I see are from that test: the files already exist (from a previous run) because they were not cleaned up. If I delete file_no_fmt.txt and file_with_fmt.txt by hand (i.e. do the job from lines 472 and 473 that you disabled on Windows), the test succeeds and leaves those files behind.

Darn. Removing the check worked for Azure because every job run is a fresh build. Just realizing makes sense it would fail running the unit tests again. I'll try to debug this in the Windows Azure build, should hopefully fix things on your desktop as well.

[agcapps]

It is certainly related. The errors I see are from that test: the files already exist (from a previous run) because they were not cleaned up. If I delete file_no_fmt.txt and file_with_fmt.txt by hand (i.e. do the job from lines 472 and 473 that you disabled on Windows), the test succeeds and leaves those files behind.

Darn. Removing the check worked for Azure because every job run is a fresh build. Just realizing makes sense it would fail running the unit tests again. I'll try to debug this in the Windows Azure build, should hopefully fix things on your desktop as well.

What is happening is that the program still has handles open to the file: they haven't been closed. I confirmed this by running Process Explorer.

On Linux, unlink() it doesn't care: it just deletes the file. If you want to delete a file on Windows you have to close all open handles, otherwise _unlink() fails.

[bmhan12]

What is happening is that the program still has handles open to the file: they haven't been closed. I confirmed this by running Process Explorer.

🤦 I didn't close the files prior to attempting to delete. Trying that now.

[rhornung67]

@bmhan12 if you have changes to make related to closing files, feel free to do that on this RC branch or on a separate PR. Whichever you prefer.

[agcapps]

So in order for axom::utilities::filesystem::Unlink() https://github.com/LLNL/axom/blob/develop/src/axom/core/utilities/FileUtilities.cpp#L126 to succeed, you will need to first call std::ofstream::close() wherever applicable.

I see that GenericOutputStream is built to deal with ostream, which is an abstraction level above ofstream. The ofstream gets closed in its destructor (https://en.cppreference.com/w/cpp/io/basic_ofstream/close, see "Notes") and the GenericOutputStream deletes its ostream in ~GenericOutputStream, so before deleting the files, we could tell slic to delete all output streams. @white238 , what do you think?

[kennyweiss]

Looks good, but I'd like to take a pass through the RELEASE_NOTES before the release.
Also, should we add a line to the RELEASE_NOTES about dropping support for XL?

(Note: I'm intentionally not approving this to ensure the branch doesn't get merged into develop)

[kennyweiss]

Do we know why there are now two lines for CMAKE_CUDA_FLAGS?

I'm pretty sure the second CMAKE_CUDA_FLAGS w/ FORCE will overwrite the first one, so it's not a problem, but it does seem a bit weird to have both.

[rhornung67]

I have no idea why there are two lines for CMAKE_CUDA_FLAGS. I just ran the scripts to generate the new host-configs.

[kennyweiss]

It's likely related to the class hierarchy for packages in Spack.
Thoughts @white238 ?

[white238]

There were some changes in spack. This was reviewed in a previous PR. The original line comes from CachedCMakePackage and the later is added by us.

[rhornung67]

A note about no longer supporting XL has been added to the release notes.

[kennyweiss]

Please update the links for axom@0.10 and axom@0.9 at the bottom of this file

[rhornung67]

Done.

[kennyweiss]

Please merge these four lines w/ the existing Changed section for axom@0.10 a few lines down (below Added)

[rhornung67]

I don't understand. This follows what is done for other releases that have updated TPL versions.

[kennyweiss]

There are two Changed sections for this release -- https://github.com/LLNL/axom/blob/b1f5bbc7c617faaf28643f0f88184ccc7d52eae2/RELEASE-NOTES.md

I can take care of the fix

[rhornung67]

I see what you mean now. I thought the TPL version updates should be up front so they are more obvious. But, whatever you prefer is fine with me.

[kennyweiss]

So in order for axom::utilities::filesystem::Unlink() https://github.com/LLNL/axom/blob/develop/src/axom/core/utilities/FileUtilities.cpp#L126 to succeed, you will need to first call std::ofstream::close() wherever applicable.

I see that GenericOutputStream is built to deal with ostream, which is an abstraction level above ofstream. The ofstream gets closed in its destructor (https://en.cppreference.com/w/cpp/io/basic_ofstream/close, see "Notes") and the GenericOutputStream deletes its ostream in ~GenericOutputStream, so before deleting the files, we could tell slic to delete all output streams. @white238 , what do you think?

Really nice find @agcapps!
Can we please add a note about this in the doxygen for removeFile?

axom/src/axom/core/utilities/FileUtilities.hpp

Lines 86 to 92 in 24751dd

	/*!
	* \brief Remove the specified file.
	* \param filename The name of the file.
	* \return 0 on success, -1 on failure. errno can obtain more information
	* about the failure.
	*/
	int removeFile(const std::string& filename);

@bmhan12 -- perhaps this should be added to #1424 ?

[kennyweiss]

@rhornung67 -- I removed the empty Unreleased section for the release. We'll add it back when we merge main to develop