ascii-boxes / boxes

Command line ASCII boxes unlimited!
https://boxes.thomasjensen.com/
GNU General Public License v3.0
613 stars 75 forks source link

Error installing/building boxes 2.2.0 under brew #118

Closed mathomp4 closed 1 year ago

mathomp4 commented 1 year ago

I seem to be having issues building/installing boxes in brew. Note that due to not having root access to my laptop, I build my homebrew stuff in $HOME/.homebrew/brew and so many codes are built from source.

To Reproduce

So this is what happens when I try to install boxes:

❯ brew install boxes
==> Downloading https://formulae.brew.sh/api/formula.jws.json
######################################################################################################################################################### 100.0%
==> Downloading https://formulae.brew.sh/api/cask.jws.json
######################################################################################################################################################### 100.0%
==> Fetching boxes
Warning: Building boxes from source as the bottle needs:
- HOMEBREW_CELLAR: /opt/homebrew/Cellar (yours is /Users/mathomp4/.homebrew/brew/Cellar)
- HOMEBREW_PREFIX: /opt/homebrew (yours is /Users/mathomp4/.homebrew/brew)
==> Downloading https://raw.githubusercontent.com/Homebrew/homebrew-core/5e5bafbda3703a7a3c8d4c9e7bd5147e8fc04453/Formula/b/boxes.rb
######################################################################################################################################################### 100.0%
==> Downloading https://github.com/ascii-boxes/boxes/archive/v2.2.0.tar.gz
==> Downloading from https://codeload.github.com/ascii-boxes/boxes/tar.gz/refs/tags/v2.2.0
 #-=O#-   #       #
==> make GLOBALCONF=/Users/mathomp4/.homebrew/brew/Cellar/boxes/2.2.0_1/share/boxes-config CC=clang YACC=/Users/mathomp4/.homebrew/brew/opt/bison/bin/bison
Last 15 lines from /Users/mathomp4/Library/Logs/Homebrew/boxes/01.make:
echo parser.o lex.yy.o cmdline.o discovery.o generate.o input.o list.o parsecode.o parsing.o query.o regulex.o remove.o shape.o tools.o unicode.o > ../out/modules.txt
/Applications/Xcode.app/Contents/Developer/usr/bin/make -C ../out -f ../src/Makefile BOXES_PLATFORM=unix ALL_OBJ="parser.o lex.yy.o boxes.o cmdline.o discovery.o generate.o input.o list.o parsecode.o parsing.o query.o regulex.o remove.o shape.o tools.o unicode.o" STRIP=true \
        CFLAGS_ADDTL="-O " flags_unix boxes
echo parser.o lex.yy.o cmdline.o discovery.o generate.o input.o list.o parsecode.o parsing.o query.o regulex.o remove.o shape.o tools.o unicode.o > ../out/modules.txt
flex --header-file=lex.yy.h ../src/lexer.l
bison --warnings=all --verbose --defines=parser.h --output=parser.c ../src/parser.y
clang -I. -I../src -Wall -W -O    -c -o parser.o parser.c
../src/parser.y:317:46: error: call to undeclared library function 'strdup' with type 'char *(const char *)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        curdes.reprules[a].search = (char *) strdup ((yyvsp[-2].s));
                                             ^
../src/parser.y:317:46: note: include the header <string.h> or explicitly provide a declaration for 'strdup'
1 error generated.
make[2]: *** [parser.o] Error 1
make[1]: *** [build] Error 2
make: *** [build] Error 2

I wonder if it's expecting some library that Clang/macOS doesn't automatically provide?

Expected behavior

Well, boxes builds. 😄

Environment (please complete the following information):

This is on macOS 13.5.1.

tsjensen commented 1 year ago

Dang, this looks like a bug to me. In parser.y, line "74.5", I really don't find #include <string.h>.

Weird that it didn't show up before. Maybe the parser/lexer somehow caused the string.h include on my machines via the generated headers.

It seems to me that the problem is gone on the current master branch because the call to strdup() is also gone. So how shall we fix it? Can you get 2.2.0 to install via brew by providing a brew-side patch? Or do you see something that we can do on the boxes side?

mathomp4 commented 1 year ago

Dang, this looks like a bug to me. In parser.y, line "74.5", I really don't find #include <string.h>.

Weird that it didn't show up before. Maybe the parser/lexer somehow caused the string.h include on my machines via the generated headers.

It seems to me that the problem is gone on the current master branch because the call to strdup() is also gone. So how shall we fix it? Can you get 2.2.0 to install via brew by providing a brew-side patch? Or do you see something that we can do on the boxes side?

Well, I am a Fortran programmer, so coding in C is a bit above me at times. 😄 It's possible it just needs another brew dependency?

So, I just now tried on my home laptop:

brew install --build-from-source --HEAD  boxes

and it failed the same:

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C ../out -f ../src/Makefile BOXES_PLATFORM=unix ALL_OBJ="parser.o lex.yy.o boxes.o bxstring.o cmdline.o discovery.o generate.o input.o list.o parsecode.o parsing.o query.o regulex.o remove.o shape.o tools.o unicode.o" STRIP=true \
        CFLAGS_ADDTL="-O " flags_unix boxes
echo parser.o lex.yy.o bxstring.o cmdline.o discovery.o generate.o input.o list.o parsecode.o parsing.o query.o regulex.o remove.o shape.o tools.o unicode.o > ../out/modules.txt
flex --header-file=lex.yy.h ../src/lexer.l
bison --warnings=all --verbose --defines=parser.h --output=parser.c ../src/parser.y
clang -I. -I../src -Wall -W -O    -c -o parser.o parser.c
clang -I. -I../src -Wall -W -O    -c -o lex.yy.o lex.yy.c
../src/lexer.l:144:26: error: call to undeclared library function 'strdup' with type 'char *(const char *)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    char *str = (char *) strdup(yytext);
                         ^
../src/lexer.l:144:26: note: include the header <string.h> or explicitly provide a declaration for 'strdup'
1 error generated.
make[2]: *** [lex.yy.o] Error 1
make[1]: *** [build] Error 2
make: *** [build] Error 2

I see something similar-ish here: https://github.com/gphoto/libgphoto2/issues/552

Can you read through that and understand it? Maybe something needs to be added to... src/config.h maybe?

tsjensen commented 1 year ago

Ah, that's a different issue, and one where I really couldn't say why it should happen, because in that case, the include statement is present in lexer.l.

Unfortunately, I still don't have access to a Mac which, afaik, would be necessary in order for me to reproduce the problem myself. Would you be able to try out a few things?

From the results, maybe we can deduce a fix. 🤞

mathomp4 commented 1 year ago

Okay. So, I did some things. I'm going to record it here so I can remember how to do it in the future. 😄

  1. I forked this repo: https://github.com/mathomp4/boxes
  2. I edited the src/config.h as seen in https://github.com/ascii-boxes/boxes/pull/119
  3. I created a homebrew/self repo: https://github.com/mathomp4/homebrew-self
  4. I copied over the boxes.rb from the "mainline" homebrew-core and then edited the urls to point to my fork
  5. I then did:
    brew tap mathomp4/self
    brew install --build-from-source --HEAD mathomp4/self/boxes

    and it seems to build and install.

tsjensen commented 1 year ago

Now, do we need a release in order to get brew back on track?

mathomp4 commented 1 year ago

Now, do we need a release in order to get brew back on track?

Yes, please, if you can make a 2.2.1 then I can make a PR to brew to update the version there.

tsjensen commented 1 year ago

Will do as soon as possible. I am currently traveling, so it may be a few days.

tsjensen commented 1 year ago

@mathomp4 Alright, we have v2.2.1 now! (Still traveling though, but I get GitHub notifications on my mobile.)