Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 3, 2025

Problem

The duckietown-shell package had a macOS compatibility issue in the check_docker_environment() function. It unconditionally called check_user_in_docker_group(), which uses the Unix grp module to verify that the user is in the "docker" group.

While this check is necessary on Linux (where Docker requires users to be in the docker group to avoid using sudo), it's problematic on macOS because:

  • Docker Desktop for Mac manages permissions differently through its application
  • There is no "docker" Unix group on macOS systems
  • The check is unnecessary and could cause issues for macOS users

Solution

This PR makes the docker group check platform-specific by adding a guard that only runs it on Linux:

Before:

def check_docker_environment():
    check_executable_exists("docker")
    check_user_in_docker_group()  # Always called
    # ... rest of function

After:

def check_docker_environment():
    check_executable_exists("docker")
    # On Linux, check if user is in the docker group
    # On macOS, Docker Desktop manages permissions differently and doesn't use groups
    if on_linux():
        check_user_in_docker_group()
    # ... rest of function

Changes

  1. Added on_macos() helper function for consistency with existing on_linux() function
  2. Made docker group check conditional by wrapping it with if on_linux(): guard
  3. Added comprehensive tests to verify platform detection works correctly
  4. Removed obsolete commented code that previously hinted at this fix but was never implemented

Testing

  • ✅ All new unit tests pass (platform detection for Linux, macOS, Windows)
  • ✅ Package imports successfully and all functions are callable
  • ✅ Code formatted with black
  • ✅ Backwards compatible - no changes to Linux behavior

Impact

  • Linux users: No change - docker group check still runs as before
  • macOS users: Docker group check is now skipped, fixing compatibility
  • Windows users: No impact (check was already not applicable)

This fix ensures the duckietown-shell package works correctly on macOS while maintaining full backwards compatibility with existing Linux installations.

Original prompt

Analyze this repository to determine if there are changes that would need to be performed to make the python package compatible with macOS.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Analyze this repository to determine if there are changes that would need to be performed to make the python package compatible with macOS. Fix macOS compatibility by making Docker group check Linux-specific Oct 3, 2025
Copilot finished work on behalf of Tuxliri October 3, 2025 12:32
Copilot AI requested a review from Tuxliri October 3, 2025 12:32
@tanij
Copy link
Member

tanij commented Nov 3, 2025

@Tuxliri where are the testing instructions?

@Tuxliri
Copy link
Contributor

Tuxliri commented Nov 4, 2025

@Tuxliri where are the testing instructions?

This is a draft PR, I have not requested a review nor testing.

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.

3 participants