FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat(stm32CubeProg): Don’t hardcode Homebrew prefix by ndoo · Pull Request #113 · stm32duino/Arduino_Tools · GitHub

/ Arduino_Tools Public

feat(stm32CubeProg): Don’t hardcode Homebrew prefix - #113

Closed
ndoo wants to merge 2 commits into
stm32duino:mainfrom
ndoo:homebrew-prefix
ConversationCommits (2)ChecksFiles changed
Closed

feat(stm32CubeProg): Don’t hardcode Homebrew prefix#113
ndoo wants to merge 2 commits into
stm32duino:mainfrom
ndoo:homebrew-prefix

Conversation

ndoo commented Dec 17, 2025

Copy link
Copy Markdown

Summary

Instead, use brew --prefix.

This PR fixes/implements the following bugs/features

  • Feature: Allows stm32CubeProg.sh to detect gnu-getopt binary/PATH when Homebrew was installed in a non-default prefix.

Validation

  • Ensure CI build is passed.
  • Demonstrate the code is solid.

Code formatting

  • Ensure AStyle check is passed thanks CI

Closing issues

N/A

feat(stm32CubeProg): Don’t hardcode Homebrew prefix
d12155e 
Instead, use `brew --prefix`.

Allows stm32CubeProg.sh to detect gnu-getopt binary/PATH when Homebrew was installed in a non-default prefix.

Signed-off-by: Andrew Yong <me@ndoo.sg>

fpistm commented Dec 17, 2025

Copy link
Copy Markdown
Member

Thanks fot the PR @ndoo
Is it fixing the #112 ?

fpistm requested review from Copilot and fpistm December 17, 2025 09:24
fpistm added the fix 🩹 Bug fix label Dec 17, 2025
fpistm added this to the 2.4.1/2.5.0 milestone Dec 17, 2025
Copilot AI reviewed Dec 17, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull request overview

This PR replaces hardcoded Homebrew prefix paths with dynamic detection using brew --prefix, enabling the script to locate the gnu-getopt binary when Homebrew is installed in non-default locations.

Key Changes:

  • Simplified conditional logic for detecting gnu-getopt from nested if-else to single check
  • Replaced hardcoded paths (/usr/local/opt and /opt/homebrew/opt) with $(brew --prefix)/opt

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

Comment thread stm32CubeProg.sh Outdated
Comment thread stm32CubeProg.sh Outdated

ndoo commented Dec 17, 2025

Copy link
Copy Markdown
Author

Hi, not really.

I am still seeing a regression when using macOS 15.6 getopt.

Though, of course, functionally I am unblocked because the solution is just that STM32Tools 2.4.0 does not like macOS 15.6 built in getopt for me.

Let me summarize on #112.

fpistm commented Dec 17, 2025

Copy link
Copy Markdown
Member

Thanks. Anyway as stated by Copilot and after some search it semms the command is not always available by default.

ndoo commented Dec 17, 2025

Copy link
Copy Markdown
Author

Thanks. Anyway as stated by Copilot and after some search it semms the command is not always available by default.

Yes, the command is only available if Homebrew is installed. Let me add a check that the command is available.

fix(stm32CubeProg): Regression for non-Homebrew gnu-getopt on macOS
136c708 
- Check for `brew` before testing prefix
- Restore previously hardcoded prefixes in case gnu-getopt was installed without Homebrew

Signed-off-by: Andrew Yong <me@ndoo.sg>

ndoo commented Dec 17, 2025

Copy link
Copy Markdown
Author

Added a commit which addresses the error if Homebrew is not present but gnu-getopt was installed in one of the previously supported prefixes (which only Homebrew seems to use anyway, but this should cover all regressions).

fpistm requested a review from Copilot December 17, 2025 09:44
Copilot AI reviewed Dec 17, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

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

Comment thread stm32CubeProg.sh

ndoo commented Dec 17, 2025

Copy link
Copy Markdown
Author

For context, Homebrew is usually how gnu-getopt is installed (looking at previous issue #100).

So I'm not sure if there is any point in hardcoding the paths, we should always use brew --prefix, the only other common way to install GNU getopt would be with MacPorts (but I’m not able to test that right now).

fpistm requested changes Dec 17, 2025
View reviewed changes
Comment thread stm32CubeProg.sh

fpistm commented Dec 17, 2025

Copy link
Copy Markdown
Member

For context, Homebrew is usually how gnu-getopt is installed (looking at previous issue #100).

So I'm not sure if there is any point in hardcoding the paths, we should always use brew --prefix, the only other common way to install GNU getopt would be with MacPorts (but I’m not able to test that right now).

Probably but what happe in brew command does not exist or failed to return prefix.
As said it is a nightmare. Each time a new OS is released there is a change. It seems the last one was in july 2025 when this version was released...

ndoo commented Dec 17, 2025

Copy link
Copy Markdown
Author

Probably but what happe in brew command does not exist or failed to return prefix.

This check should cover your concern

if command -v brew >/dev/null 2>&1 &&

I have tested by renaming brew executable from brew to brew2 and this condition successfully fails (haha), resulting in moving on to the next conditions that test the previous hardcoded paths.

ndoo requested a review from fpistm December 17, 2025 10:05
fpistm added the abandoned No more work on this label Jul 22, 2026

fpistm commented Jul 22, 2026

Copy link
Copy Markdown
Member

Superseded by #116.

fpistm closed this Jul 22, 2026
fpistm mentioned this pull request Jul 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
fpistm Awaiting requested review from fpistm

Assignees

No one assigned

Labels

abandoned No more work on this fix 🩹 Bug fix

Projects

None yet

Milestone

2.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL