qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
17.98k stars 38.65k forks source link

[Bug] `./util/docker_build.sh` should not require `docker-machine` #24273

Closed puchupala closed 4 weeks ago

puchupala commented 4 weeks ago

Describe the Bug

I used to be able to ./util/docker_build.sh on my Mac without having to install docker-machine. However, since https://github.com/qmk/qmk_firmware/pull/23988 make ./util/docker_build.sh call ./util/docker_cmd.sh, it is now also (seemingly unnecessary) require docker-machine as well.

I tried commenting out docker-machine checking code and ./util/docker_build.sh still ran and built my firmware just fine, suggesting that it's not necessary for building.

Keyboard Used

No response

Link to product page (if applicable)

No response

Operating System

MacOS

qmk doctor Output

No response

Is AutoHotKey / Karabiner installed

Other keyboard-related software installed

No response

Additional Context

No response

tzarc commented 4 weeks ago

You'll find that the docker-machine check has been present for much longer than the PR you've quoted.

It's there to enable flashing -- historically we've had support requests as to why flashing doesn't work with docker builds, so I'd imagine this is why that check is present.

Feel free to raise a PR adding a new environment variable or similar mechanism to disable the check -- it'll need to stay on by default.

puchupala commented 4 weeks ago

You'll find that the docker-machine check has been present for much longer than the PR you've quoted.

@tzarc My apologies. I was using ZSA's fork on firmware23 branch, which doesn't have docker-machine check in ./util/docker_build.sh.

Feel free to raise a PR adding a new environment variable or similar mechanism to disable the check -- it'll need to stay on by default.

I'll get to that. Thank you very much for the suggestion.