Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gptfdisk 1.0.7 #76593

Closed
wants to merge 3 commits into from
Closed

gptfdisk 1.0.7 #76593

wants to merge 3 commits into from

Conversation

chenrui333
Copy link
Member

Created with brew bump-formula-pr.

@chenrui333
Copy link
Member Author 

clang: error: no such file or directory: '/usr/local/Cellar/ncurses/6.2/lib/libncurses.dylib'
make: *** [cgdisk] Error 1

@chenrui333 chenrui333 added the build failure CI fails while building the software label May 4, 2021
@carlocab
Copy link
Member

carlocab commented May 4, 2021

Looks like they switched from looking for /usr/lib/libncurses.dylib to looking for Homebrew libncurses.dylib. Except they also hardcoded the prefix.

@chenrui333 chenrui333 closed this May 8, 2021
chenrui333 and others added 3 commits May 26, 2021 22:23
@cho-m cho-m reopened this May 27, 2021
@BrewTestBot BrewTestBot added the bump-formula-pr PR was created using `brew bump-formula-pr` label May 27, 2021
@cho-m cho-m added ready to merge PR can be merged once CI is green and removed build failure CI fails while building the software labels May 27, 2021
SMillerDev
SMillerDev reviewed May 27, 2021
View reviewed changes
Formula/gptfdisk.rb
# $(CXX) $(LIB_OBJS) -L/usr/lib -licucore gpttext.o gdisk.o -o gdisk

cgdisk: $(LIB_OBJS) cgdisk.o gptcurses.o
- $(CXX) $(LIB_OBJS) cgdisk.o gptcurses.o /usr/lib/libncurses.dylib $(LDFLAGS) $(FATBINFLAGS) -o cgdisk
+ $(CXX) $(LIB_OBJS) cgdisk.o gptcurses.o -L/usr/lib -lncurses $(LDFLAGS) $(FATBINFLAGS) -o cgdisk
- $(CXX) $(LIB_OBJS) cgdisk.o gptcurses.o /usr/local/Cellar/ncurses/6.2/lib/libncurses.dylib $(LDFLAGS) -o cgdisk
Copy link
Member

Choose a reason for hiding this comment

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

Can't this link to the opt_prefix?

Copy link
Member

Choose a reason for hiding this comment

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

Both the previous diff (used in v1.0.6) and new diff here (v1.0.7) are patching Makefile to use macOS ncurses, corresponding to the uses_from_macos "ncurses"

In this case, the diff is doing:
/usr/local/Cellar/ncurses/6.2/lib/libncurses.dylib ==> -L/usr/lib -lncurses

The alternative would be to use Homebrew's ncurses along with corresponding path.

Copy link
Member

Choose a reason for hiding this comment

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

Damned, misread the patch. + - at the start of the line is confusing

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@github-actions github-actions bot added the outdated PR was locked due to age label Jun 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 27, 2021
@chenrui333 chenrui333 deleted the bump-gptfdisk-1.0.7 branch December 18, 2022 05:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SMillerDev SMillerDev SMillerDev left review comments

@cho-m cho-m cho-m left review comments

@carlocab carlocab carlocab approved these changes

Assignees
No one assigned
Labels
bump-formula-pr PR was created using `brew bump-formula-pr` outdated PR was locked due to age ready to merge PR can be merged once CI is green
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@chenrui333 @carlocab @BrewTestBot @SMillerDev @cho-m