Skip to content

fix: remove hardcoded sudo docker and respect DOCKEREXE override in mkconcore.py (fixes #343)#400

Closed
GaneshPatil7517 wants to merge 1 commit into
ControlCore-Project:devfrom
GaneshPatil7517:fix/dockerexe-config-respected
Closed

fix: remove hardcoded sudo docker and respect DOCKEREXE override in mkconcore.py (fixes #343)#400
GaneshPatil7517 wants to merge 1 commit into
ControlCore-Project:devfrom
GaneshPatil7517:fix/dockerexe-config-respected

Conversation

@GaneshPatil7517
Copy link
Copy Markdown
Contributor

Summary

This PR resolves Issue #343 by removing hardcoded "sudo docker" usage in mkconcore.py and ensuring the DOCKEREXE configuration is respected consistently.

Problem

  • mkconcore.py defaulted to "sudo docker", which fails on:
    • Docker Desktop (macOS/Windows) where sudo isn't needed
    • Rootless Docker setups
    • Podman environments
  • Generated maxtime, params, and unlock scripts all hardcoded "sudo docker" instead of using the DOCKEREXE variable

Changes Made

  • Default DOCKEREXE: Changed from "sudo docker" to os.environ.get("DOCKEREXE", "docker") — defaults to "docker", overridable via environment variable
  • concore.sudo: Still respected — if concore.sudo exists, it overrides DOCKEREXE as before
  • Generated scripts: All 18 hardcoded "sudo docker" occurrences in maxtime, params, and unlock script generation replaced with f-string {DOCKEREXE} references

Benefits

  • Works out-of-the-box on macOS/Windows Docker Desktop
  • Supports rootless Docker installations
  • Supports Podman (DOCKEREXE=podman)
  • Respects user configuration via env var or concore.sudo
  • Improves cross-platform portability

Backward Compatibility

Users who previously relied on the default sudo docker behavior can restore it by either:

  • Setting DOCKEREXE="sudo docker" environment variable
  • Using concore.sudo config file (unchanged behavior)

Scope

  • Single file modification: mkconcore.py
  • No changes to concore-lite or Verilog files

Testing

  • Python syntax validated
  • All 77 existing tests pass
  • Verified: default uses docker, env override works, concore.sudo override works, no hardcoded sudo in generated scripts

Copilot AI review requested due to automatic review settings February 18, 2026 08:19
Copy link
Copy Markdown
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 addresses Issue #343 by removing the hardcoded "sudo docker" default in mkconcore.py and ensuring the configured Docker command (DOCKEREXE) is used consistently when generating helper scripts.

Changes:

  • Change the default Docker command from "sudo docker" to os.environ.get("DOCKEREXE", "docker").
  • Keep existing precedence where concore.sudo (if present) overrides the environment/default value.
  • Update generated maxtime, params, and unlock scripts to use DOCKEREXE instead of hardcoded "sudo docker".

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

@pradeeban
Copy link
Copy Markdown
Member

Looks like a lot of lines are changed (perhaps a change in linebreak/EoL character), when the actual change is fewer lines. Can you please re-create the PR with just the necessary minimal line changes?

@GaneshPatil7517
Copy link
Copy Markdown
Contributor Author

ok @pradeeban Sir give me few minutes .......

@GaneshPatil7517 GaneshPatil7517 deleted the fix/dockerexe-config-respected branch February 19, 2026 06:14
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