Skip to content

Conversation

@mwqgithub
Copy link
Contributor

@mwqgithub mwqgithub commented Dec 11, 2025

Purpose of the change

Support configuration script in older macos systems. Avoid installing xcode

Description

In older macos systems like macos-12, homebrew doesn't provide pre-compiled jdk and jdk has to be compiled locally when installing neo4j. This makes the installation of neo4j depends on Xcode, which is very large.
So we would like to switch to download the pre-built java binary and neo4j binary directly and extract them. Like we do in linux and windows.

Fixes/Closes

Fixes #758

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Project Maintenance (updates to build scripts, CI, etc., that do not affect the main project)

How Has This Been Tested?

  • Unit Test
  • Integration Test
  • Manual verification (list step-by-step instructions)

Checklist

  • I have signed the commit(s) within this pull request
  • My code follows the style guidelines of this project (See STYLE_GUIDE.md)
  • I have performed a self-review of my own code
  • I have commented my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Maintainer Checklist

  • Confirmed all checks passed
  • Contributor has signed the commit(s)
  • Reviewed the code
  • Run, Tested, and Verified the change(s) work as expected

@mwqgithub mwqgithub force-pushed the fix-macos-installation branch 4 times, most recently from f24c001 to 61ccb0f Compare December 11, 2025 09:57
@mwqgithub mwqgithub requested a review from jealous December 12, 2025 01:31
@jealous jealous requested review from a team, Charlie-Yi-2002, Copilot and jgong December 12, 2025 18:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies the macOS installation process to use pre-built binaries downloaded directly via zip/tar files instead of relying on Homebrew, eliminating the dependency on Xcode in older macOS systems like macOS-12.

Key changes:

  • Replaces Homebrew-based Neo4j installation with direct binary downloads for macOS
  • Adds support for both ARM64 and x86_64 architectures on macOS
  • Updates test infrastructure to verify installation on both macOS architectures

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/memmachine/installation/utilities.py Adds macOS JDK download URLs for both ARM64 and x64 architectures
src/memmachine/installation/memmachine_configure.py Refactors MacosEnvironment to inherit from LinuxEnvironment and implements manual binary installation instead of Homebrew
tests/memmachine/installation/test_memmachine_configure.py Updates mock environment and adds test coverage for both ARM64 and x64 macOS installations
.github/workflows/installation-test.yml Adds macos-15-intel to the test matrix and updates conditional logic for macOS-specific steps

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

neo4j_tar_path = str(Path(self.install_dir, LINUX_NEO4J_TAR_NAME))
self.environment.download_file(LINUX_NEO4J_URL, neo4j_tar_path)
self.environment.extract_tar(neo4j_tar_path, self.install_dir)
neo4j_dir = str(Path(self.install_dir) / "neo4j-community-2025.09.0")
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The Neo4j version is hardcoded here but defined as NEO4J_DIR_NAME in utilities.py. Use NEO4J_DIR_NAME instead to maintain consistency and avoid duplication. This should be: neo4j_dir = str(Path(self.install_dir) / NEO4J_DIR_NAME)

Suggested change
neo4j_dir = str(Path(self.install_dir) / "neo4j-community-2025.09.0")
neo4j_dir = str(Path(self.install_dir) / NEO4J_DIR_NAME)

Copilot uses AI. Check for mistakes.
Comment on lines 166 to 167
def extract_tar(self, zip_path: str, extract_to: str) -> None:
self.extracted_files[zip_path] = extract_to
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The parameter name 'zip_path' is misleading since this method extracts tar files, not zip files. Rename it to 'tar_path' to accurately reflect the file type being processed.

Suggested change
def extract_tar(self, zip_path: str, extract_to: str) -> None:
self.extracted_files[zip_path] = extract_to
def extract_tar(self, tar_path: str, extract_to: str) -> None:
self.extracted_files[tar_path] = extract_to

Copilot uses AI. Check for mistakes.
#
- name: macOS - Install Ollama
if: matrix.os == 'macos-latest'
if: matrix.os == 'macos-latest' or matrix.os == 'macos-15-intel'
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The conditional expression uses Python syntax ('or') instead of GitHub Actions YAML syntax. This should use '||' for proper workflow evaluation: if: matrix.os == 'macos-latest' || matrix.os == 'macos-15-intel'

Copilot uses AI. Check for mistakes.
- name: macOS - Test installation
if: matrix.os == 'macos-latest'
if: matrix.os == 'macos-latest' or matrix.os == 'macos-15-intel'
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The conditional expression uses Python syntax ('or') instead of GitHub Actions YAML syntax. This should use '||' for proper workflow evaluation: if: matrix.os == 'macos-latest' || matrix.os == 'macos-15-intel'

Suggested change
if: matrix.os == 'macos-latest' or matrix.os == 'macos-15-intel'
if: matrix.os == 'macos-latest' || matrix.os == 'macos-15-intel'

Copilot uses AI. Check for mistakes.
@jealous
Copy link
Contributor

jealous commented Dec 13, 2025

@mwqgithub You may need to check if those copilot comments make sense.

@mwqgithub
Copy link
Contributor Author

@mwqgithub You may need to check if those copilot comments make sense.

Yes, I think these comments are very useful. Let me fix them.

@mwqgithub mwqgithub force-pushed the fix-macos-installation branch from 048fefc to 81190cf Compare December 15, 2025 02:27
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.

[Bug]: Error when trying to use the configuration wizard to install Neo4j when on MacOS 12.2

2 participants