Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix builds for macos 12.6 #18642

Closed
wants to merge 13 commits into from
Closed

Conversation

markcmiller86
Copy link
Member

Description

Resolves #

Type of change

  • Bug fix~~
  • New feature~~
  • Documentation update~~
  • Other~~

How Has This Been Tested?

Reminders:

  • Please follow the style guidelines of this project.
  • Please perform a self-review of your code before submitting a PR and asking others to review it.
  • Please assign reviewers (see VisIt's PR procedures for more information).

Checklist:

  • I have commented my code where applicable.~~
  • I have updated the release notes.~~
  • I have made corresponding changes to the documentation.~~
  • I have added debugging support to my changes.~~
  • I have added tests that prove my fix is effective or that my feature works.~~
  • I have confirmed new and existing unit tests pass locally with my changes.~~
  • I have added new baselines for any new tests to the repo.~~
  • I have NOT made any changes to protocol or public interfaces in an RC branch.~~

@@ -2346,7 +2346,7 @@ IF (NOT WIN32)
SET (CPACK_BUNDLE_PLIST "${VISIT_BINARY_DIR}/tools/dev/scripts/Info.plist")
SET (CPACK_BUNDLE_STARTUP_COMMAND "${VISIT_BINARY_DIR}/exe/visit_macos_launcher")
ELSE(APPLE AND VISIT_CREATE_APPBUNDLE_PACKAGE)
SET(CPACK_GENERATOR "TGZ")
SET(CPACK_GENERATOR "TXZ")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change leads to 100Mb smaller tarball on mac. Would assume similar savings on other platforms. If we could control flags to xz, we could improve that a bit more. We could do that if we used Python tools to tar and compress...as we do in the top level data dir.

cmd = "hdiutil create -srcFolder %s -o %s" % (src_folder, temp_dmg)
# ULMO format (xz) is 1/2 the size of default format (UDZO which is zlib level 1)
#cmd = "hdiutil create -srcFolder %s -o %s" % (src_folder, temp_dmg)
cmd = "hdiutil create -format ULMO -srcFolder %s -o %s" % (src_folder, temp_dmg)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change leads to 275Mb smaller .dmg file on mac

export MPICH_URL=${MPICH_URL:-http://www.mpich.org/static/tarballs/${MPICH_VERSION}}
export MPICH_MD5_CHECKSUM="9ed4cabd3fb86525427454381b25f6af"
export MPICH_SHA256_CHECKSUM="fe551ef29c8eea8978f679484441ed8bb1d943f6ad25b63c235d4b9243d551e5"
if [[ "$OPSYS" == "Darwin" ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating mpich with newer libtool involves quite a bit. No easy way to just patch it. So, I created a new third-party asset named mpich-3.3.1-libtool-2.4.6 which expands also to mpich-3.3.1 (honestly should be harmless if its used on non-macOS systems too).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit is needed for macOS 12.6. So I cherry picked it to #19293

src/tools/dev/scripts/bv_support/bv_mpich.sh Show resolved Hide resolved
sed -i orig -e 's/^main()/int main()/' configure
else
sed -i.orig -e 's/^main()/int main()/' configure
fi
info "Invoking command to configure AdvIO"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many try-run blocks for sizes of types in configure defined a main() with no type which causes modern compilers to fail outright. This sed command fixes them. Its a little different on macOS then other *nix systems. Windows?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markcmiller86 Sorry, didn't see the question about Windows sooner. Not using the build_visit script on Windows, so not an issue.

@markcmiller86 markcmiller86 marked this pull request as ready for review April 14, 2023 18:28
@markcmiller86
Copy link
Member Author

@biagas and @brugger1 I've pulled this out of DRAFT mode. I think it is about ready.

brugger1
brugger1 previously approved these changes Apr 14, 2023
@markcmiller86
Copy link
Member Author

Ok, so my attempt to build from a fresh state for this branch did fail in Qt. And I need to investigate and fix.

@markcmiller86
Copy link
Member Author

The problem with Qt5 on macOS is that it does not support python3. It requires a python2 (for at least QtWebEngine anyways...maybe we should explicitly disable that?). On my macOS 12.6.7, with a vanilla PATH, I have a python3 but no python or python2 and that is what Qt is looking for. Now, I have a version 2 python there in /opt/local/bin. But, there is a ton of other stuff in /opt/local/bin. So, I don't want /opt/local/bin in my path. So, I added a symlink named python in a random dir and added that dir to my path so hopefully it will find that version 2 python

@markcmiller86
Copy link
Member Author

I wound up having to add -skip webengine to Qt configure.

@brugger1
Copy link
Collaborator

There is a patch in bv_qt.sh that should handle the python issue, but it is only applied for Qt 5.10.1. It looks like it should also be added for Qt 5.14.2.

#
# If python doesn't exist, substitute python2 for python.
#
python -c "print(1)"
if [[ $? != 0 ]] ; then
info "Patching qt 5.10.1 for python to python2"
cd qtdeclarative
find . -type f -exec sed -i "s/python/python2/g" {} \;
cd ..
fi

@cyrush
Copy link
Member

cyrush commented Jan 11, 2024

PR #18848 will include the changes for macOS.

@cyrush
Copy link
Member

cyrush commented Feb 13, 2024

closing in lieu of PR #18848 and #19293

@cyrush cyrush closed this Feb 13, 2024
@markcmiller86 markcmiller86 deleted the rel-mcm86-11apr23-build-fixes branch May 4, 2024 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants