[Chapel Merge] Move override-ld to printchplenv

Notifications Commits
1

Branch: refs/heads/main
Revision: 9f68b2b
Author: mppf
Link: Move override-ld to printchplenv by mppf · Pull Request #20385 · chapel-lang/chapel · GitHub
Log Message:

Merge pull request #20385 from mppf/move-override-ld-printchplenv

Move override-ld to printchplenv

This PR addresses a future work item left by PR #18880. In particular,
historically we have saved the linker to use in a file override-ld and
then the LLVM backend reads this file and uses the selected linker in
order to link the binary. We recently observed some problems with this
strategy in the context of Homebrew. The Homebrew Chapel package had this
override-ld file referring to clang in a particular location; but the
Homebrew clang package was upgraded and clang was no longer available at
that location. (See also
chapel: resolve failures in mac m1 by bhavanijayakumaran · Pull Request #107405 · Homebrew/homebrew-core · GitHub ).

So, this PR moves the override-ld logic to something supported by the
chplenv Python scripts. One wrinkle with that is that the Makefile logic
was using GASNET_LD_REQUIRES_MPI but that variable is not stored in the
Gasnet .pc file that is used by the chplenv scripts in LLVM compilation.
So, this PR uses the approximation of determining
GASNET_LD_REQUIRES_MPI by searching for mpi in the GASNET_LD
command name.

Additionally, while debugging the issue, I noticed some odd things about
the error handling within executeAndWait, so this PR cleans that
function up a bit as well. It also removes some arguments to that
function that aren't really necessary. It adds a C++ dyno test of this
function to make sure the return value of the function is 0 when the
command runs and something other than 0 when the command does not exist.

The chpl-env-gen.h needed some adjustment to work with this change so I
tidied it up to stop filtering out environment variables that often have
characters that can't be in a #define and instead to use a more
comprehensive pattern when replacing such characters with _.

Future Work:

  • include Bitbucket in
    our bundled GASNet to actually use GASNET_LD_REQUIRES_MPI in the
    logic to match the previous behavior.

Reviewed by @arezaii

  • full local testing

  • XC gasnet+aries testing

  • XC ugni testing

  • XC mpi testing

  • Mac OS X testing

  • testing Hello world on a real ibv system

    Modified Files:
    A compiler/dyno/test/util/testSubprocess.cpp
    M compiler/dyno/include/chpl/util/subprocess.h
    M compiler/dyno/lib/util/subprocess.cpp
    M compiler/dyno/test/util/CMakeLists.txt
    M compiler/include/driver.h
    M compiler/llvm/clangUtil.cpp
    M compiler/main/driver.cpp
    M compiler/util/mysystem.cpp
    M runtime/Makefile
    M test/runtime/sungeun/chpl-env-gen.precomp
    M util/chplenv/chpl_gasnet.py
    M util/chplenv/compile_link_args_utils.py
    M util/chplenv/overrides.py
    M util/chplenv/printchplenv.py
    M util/chplenv/third_party_utils.py

    Compare: Comparing 454cdd06f2d3...9f68b2b2dac4 · chapel-lang/chapel · GitHub