hexagonal-sun / bic

A C interpreter and API explorer.
GNU General Public License v2.0
815 stars 36 forks source link

Native macOS support? #25

Closed zbeekman closed 4 years ago

zbeekman commented 5 years ago

Hi, I know that I can run this using docker on macOS, but, once you hit a stable release it would be awesome to package this with homebrew.

My current testing shows that:

Here are logs with information about the build steps, environment and test failures: bic.zip

In particular:

hexagonal-sun commented 5 years ago

Hi there,

Thanks for the report and your logs, very useful!

The code has been written with MacOS support in mind, however I've been lax in verifying that recent changes haven't broken things. I've just pushed some changes that should help and after running make check with the new code I have 61 tests passing and 7 failing. Could you please confirm this on your end?

These failing tests look as though they are to do with parsing rules that I haven't yet introduced into bic; I'll look to fix these over the next few days.

One thing that I have noticed: when configuring the package, I need to use homebrew's version of bison. Even if this is installed, it doesn't look as though it is used by default and as such my configure line looks like:

YACC="/usr/local/Cellar/bison/3.4/bin/bison -y" ./configure --enable-debug

Hope that helps!

zbeekman commented 5 years ago

Sorry for the slow reply. Been busy w/ work. Ping me next week if I still haven’t responded.

zbeekman commented 5 years ago

One thing that I have noticed: when configuring the package, I need to use homebrew's version of bison. Even if this is installed, it doesn't look as though it is used by default and as such my configure line looks like:

Yes, bison is provided by macOS, so it is keg only. If the native bison is too old then homebrew's bison is the best bet. But use it you may need to follow the directions from the caveat:

If you need to have bison first in your PATH run:
  echo 'export PATH="/usr/local/opt/bison/bin:$PATH"' >> ~/.bash_profile

For compilers to find bison you may need to set:
  export LDFLAGS="-L/usr/local/opt/bison/lib"

There is also Berkeley Yacc (byacc) available through homebrew, and this get's linked because it provides the byacc binary rather than yacc which conflicts with system bison/yacc.

zbeekman commented 5 years ago

A few additional notes:

  1. You should recommend:

    YACC="$(brew --prefix bison)/bin/bison -y" ./configure --enable-debug

    This will allow bison to be upgraded w/o breaking the configure script.

  2. Your build system requires an in-source build. While this is fairly common with autotools projects, I always try to build in a separate directory to make cleanup easier.

  3. Autoreconf -i had some suggestions for you:

    $ autoreconf -i
    glibtoolize: putting auxiliary files in '.'.
    glibtoolize: copying file './ltmain.sh'
    glibtoolize: Consider adding 'AC_CONFIG_MACRO_DIRS([m4])' to configure.ac,
    glibtoolize: and rerunning glibtoolize and aclocal.
    glibtoolize: Consider adding '-I m4' to ACLOCAL_AMFLAGS in Makefile.am.
    glibtoolize: 'AC_PROG_RANLIB' is rendered obsolete by 'LT_INIT'
  4. Trying to build with GCC 9 fails during configure: checking for __gmpz_init in -lgmp... no but gmp is installed and is at version 6.1.2. Experimenting with LT_SYS_LIBRARY_PATH and --with-sys-root didn't seem to change this problem. config.log

  5. Using the system compilers (clang & clang++) works with a lot of warning, and produces the following test results (test-suite.log):

    ============================================================================
    Testsuite summary for bic v0.9.1
    ============================================================================
    # TOTAL: 68
    # PASS:  61
    # SKIP:  0
    # XFAIL: 0
    # FAIL:  7
    # XPASS: 0
    # ERROR: 0
    ============================================================================
    See testsuite/test-suite.log
    Please report to dev@mattleach.net
    ============================================================================

You can leave this open to track the test failures, or close it, up to you. If you can get this to a somewhat stable place for macOS (you probably should get some macOS CI testing happening) and resolve the test failures, then I'll add it to Homebrew after the first stable release.

zbeekman commented 4 years ago

@hexagonal-sun Any chance you've been able to take a look at my most recent comments?

hexagonal-sun commented 4 years ago

@zbeekman, apologies for the late reply on this, and many thanks for the info posted above. Please see my replies for each of your points:

1

Thanks for the info - I didn't know about brew --prefix. I'll update README.org with your recommended instructions (or feel free to submit a PR).

2

Hm, interesting. Out-of-tree builds seem to work for me. I used the following commands:

$ cd Development/bic
$ mkdir build
$ cd build
$ ../configure
$ make && make check

Where did you run into trouble?

3

I'll look at implementing those. I think I've had trouble with it breaking the build on older systems, this was a few years ago though.

4

I'll look into that - I'm not sure why it wouldn't work with gcc on MacOS. Out of interest, how did you get it to use gcc instead of clang? It looks as though you only did a normal ./configure.

5

I've been doing some digging around as to why you are seeing those seven failures on the testsuite. I think the way I've parsed declarations is incorrect. When I run the testcase manually I get:

Parser Error: /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/signal.h:390 syntax error, unexpected '(', expecting ')'.

Which means bic isn't able to parse the declaration:

    void(*signal(int, void (*)(int)))(int);

I'm currently looking into refactoring the parser to handle the above declarations.

hexagonal-sun commented 4 years ago

Hey @zbeekman ,

I've been doing quite a bit of work getting MacOS supported and I've now got a PR ( #34 ) ready to get merged. If you could take a look and let me know what you think, that would be great!

Thanks, Matt

hexagonal-sun commented 4 years ago

I've merged #34 and am nearing to a full first release of bic once I've cleaned up the code a little bit and reduced some of the warnings generated.

@zbeekman , if you're still okay creating a homebrew package for bic that would be great! Until then, I'll keep this issue open.

hexagonal-sun commented 4 years ago

34 has now been merged which brings MacOS support into CI. Therefore closing this PR.

zbeekman commented 4 years ago

Sorry for the radio silence. Drowning in work as a big contract comes to an end this month, and hunting for a new office since the building where my office is currently located is being sold out from under us.

I'll take a look, and if it's low-hanging fruit, submit a PR to add to homebrew. Super excited to try the CLI/REPL out myself and see this in action. Thanks for your work on macOS support!

hexagonal-sun commented 4 years ago

Ah no problem, understand that life gets in the way sometimes!

Having this in homebrew would be great! If you'd like to wait until v1.0 is released that would be good. I only have #37 to finish and a bit of cleanup then I'll do a release.

Thanks for your help with the MacOS port!

zbeekman commented 4 years ago

yeah it needs an official release and a stable version for homebrew-core, so my plan was test now, report any issues, and get a draft formula written. Then update it, re-check it at 1.0 and submit to homebrew-core

zbeekman commented 4 years ago

@hexagonal-sun Is there anyway you could coerce the build to use macOS' native bison?

That would go a long way towards acceptance in homebrew and dropping the bison dependency. The error it dies on is:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/bison: unrecognized option `--warnings=no-yacc'

which looks like it could be safely squelched in the build system (at least on macOS)

EDIT: This line: https://github.com/hexagonal-sun/bic/blob/23c7728c831026c5ffbd0e5e1eb6f425582fc200/src/Makefile.am#L5

zbeekman commented 4 years ago

@hexagonal-sun another Homebrew related question, since I'm looking at this:

My knowledge of auto-tools is much hazier than CMake. Is my understanding here correct about the following dependencies:

The ones I'm most unsure about are libtool and autoconf-archive.

FYI bic is perfectly happy to use macOS' flex it appears, as well as the native macOS clang tooling.

zbeekman commented 4 years ago

Update: It appears that the bison/yacc that ships with XCode is not acceptable for compiling bic. I tried disabling the warning flag and got:

Making install in src
  GEN      replparser.y
  GEN      repllex.l
  GEN      lang_lexer.h
  CXX      lang.o
  LEX      lang_lexer.cpp
  CXX      gentypes.o
  CXX      genctypes.o
  CXX      gentree.o
  CXX      genaccess.o
  GEN      cscriptparser.y
  GEN      cscriptlex.l
  YACC     replparser.c
  GEN      repllex.h
  CXX      lang_lexer.o
/private/tmp/bic-20200211-37955-5fbwn9/src/replparser.y:68.9-19: syntax error, unexpected identifier, expecting string

As I mentioned above, if there's a way to make this use XCode's bison/yacc that will lower a non-trivial barrier to entry into homebrew-core. If the functionality of XCode's bison/yacc is just so old that it's unreasonable to work around then that is a valid case for making an exception.