mathomp4 commented 1 year ago

This is attempting to add macOS in the CI.

mathomp4 commented 1 year ago

Yeah, I'll definitely need @tsjensen to help with this. I know I'm doing something dumb... :)

tsjensen commented 1 year ago

121

mathomp4 commented 1 year ago

Well, I've gotten closer. First, I had to install the GNU versions of sed, grep, and xargs since the BSD ones that macOS has by default are odd and your tests use GNU options (and I don't mind this because I hate the BSD versions of those utilities).

But even with that, as far as I can tell the white-box tests can't run at the moment since macOS ld does not support --wrap and I'm not sure how to get around that. Frankly, I'm not sure what it's doing. 😄

Now the black-box tests get pretty much through everything...but dies at the end with an error. I can see one test fails for Unicode/language-y (LC_ALL) like issues:

Running test case: 111_manual_encoding_iso.txt
tr: Illegal byte sequence
tr: Illegal byte sequence
    Invoking: boxes -ac -n ISO_8859-15
geninfo: WARNING: RC option 'lcov_branch_coverage' is deprecated.  Consider using 'branch_coverage. instead.  (Backward-compatible support will be removed in the future
    Coverage: 37.5%
Error in test case: 111_manual_encoding_iso.txt (top: actual; bottom: expected)
1,3c1,2
< /****/
< /*  */
< /****/
\ No newline at end of file
---
>     /**************/
>     /*     
\ No newline at end of file

Internet said setting LC_ALL=C would help and 111 passed, but then more Unicode tests failed that didn't before, so I reverted that. Maybe if there was a way to "if APPLE, don't run test 111?"

I also see:

lcov: ERROR: 'exclude' pattern '*/lex.yy.c' is unused.
    (use "lcov --ignore-errors unused ..." to bypass this error)

and then finally:

make: *** [covtest] Error 1

tsjensen commented 1 year ago

That's great progress! Here's my 2 cents on some of the topics:

Test 111: This one is a little special because the .txt file describing the test case is in a different encoding than the others. Maybe you could check that the encoding is really ISO_8859-15 when the test is run? (It depends on Git, which should make sure the encoding is correct.)
If you need to exclude a black-box test, just rm it from the test folder before execution. That would be the last resort, of course.
The lcov: ERROR: 'exclude' pattern '*/lex.yy.c' is unused. is from the white-box tests in report.sh. This is a follow-up problem from the linker problem.
So, the main problem is the linker problem (´--wrap´). It allows the unit tests to run and test output made by overriding the print function (Michael Feathers would call this a "link seam"). I think you found a great resource on that already. On the linked page, several tools are suggested that could be used to solve the problem. Especially, getting "half way" would be enough, because we don't call the original function. But since I have no Mac, I'm afraid I can't be much help unraveling this. 😢

chorpler commented 12 months ago

Don't know if this is helpful, or if you've already worked this out, but on my M1 Mac Mini, I tried to figure out if there was a way to simulate the --wrap flag that Gnu ld supports but Apple's ld doesn't. I couldn't get anything to work. Lots of hints and suggestions found by googling, but nothing that worked.

Finally, I decided to try the "nuclear option", and was able to get the unit tests to pass by defining a function pointer bx_fprintf_ptr in src/tools.c and initializing it to point to the normal bx_fprintf() function. I also added a function to src/tools.c called set_bx_fprintf() which, when called, will set bx_fprintf_ptr to the function passed in as parameter 1. Then, when the setup_mocks() function from utest/global_mocks.c is called at the start of unit testing, it replaces all the calls to bx_fprintf() with calls to __wrap_bx_fprintf() by adding set_bx_fprintf(__wrap_bx_fprintf); as the last statement in the setup_mocks() function.

(I also added a section in Makefile to see if it's being built on a Mac, and defined a separate flags_osx target in addition to the flags_unix target in utest/Makefile, which is identical to flags_unix but with the $(foreach MOCK,$(MOCKS),-Wl,--wrap=$(MOCK)) statement removed:

flags_osx:
    $(eval CFLAGS := -I. -I$(SRC_DIR) -O -Wall -W -Wno-stringop-overflow $(CFLAGS_ADDTL))
    $(eval LDFLAGS := $(LDFLAGS) --coverage $(LDFLAGS_ADDTL))
    $(eval UTEST_EXECUTABLE_NAME := unittest)
    $(eval UTEST_OBJ := $(UTEST_NORM:.c=.o))

If this is helpful, I'd be glad to submit a pull request with my changes (or maybe an update this pull request? ... I'm not sure of the proper way to do this kind of thing).

Here's the output of make utest after make clean && make were already run:

nano:boxes admin$ make utest
Building for MacOS
make[1]: Entering directory '/Users/local/src/boxes/utest'
make -C ../out -f ../utest/Makefile BOXES_PLATFORM=osx UTEST_OBJ="global_mock.o bxstring_test.o cmdline_test.o tools_test.o regulex_test.o remove_test.o main.o unicode_test.o utest_tools.o" \
    CFLAGS_ADDTL="" flags_osx unittest
make[2]: Entering directory '/Users/local/src/boxes/out'
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o global_mock.o ../utest/global_mock.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o bxstring_test.o ../utest/bxstring_test.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o cmdline_test.o ../utest/cmdline_test.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o tools_test.o ../utest/tools_test.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o regulex_test.o ../utest/regulex_test.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o remove_test.o ../utest/remove_test.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o main.o ../utest/main.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o unicode_test.o ../utest/unicode_test.c
gcc -I. -I../src -O -Wall -W -Wno-stringop-overflow  -I/opt/homebrew/include -L/opt/homebrew/lib  -c -o utest_tools.o ../utest/utest_tools.c
gcc --coverage  global_mock.o bxstring_test.o cmdline_test.o tools_test.o regulex_test.o remove_test.o main.o unicode_test.o utest_tools.o parser.o lex.yy.o bxstring.o cmdline.o detect.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 -o unittest -lunistring -lpcre2-32 -lcmocka
make[2]: Leaving directory '/Users/local/src/boxes/out'
rm -f ../out/*.gcda
cd ../out ; ./unittest
[==========] cmdline_tests: Running 35 test(s).
[ RUN      ] test_indentmode_none
[       OK ] test_indentmode_none
[ RUN      ] test_indentmode_invalid_long
[       OK ] test_indentmode_invalid_long
[ RUN      ] test_indentmode_invalid_short
[       OK ] test_indentmode_invalid_short
[ RUN      ] test_indentmode_box
[       OK ] test_indentmode_box
[ RUN      ] test_indentmode_text
[       OK ] test_indentmode_text
[ RUN      ] test_killblank_true
[       OK ] test_killblank_true
[ RUN      ] test_killblank_false
[       OK ] test_killblank_false
[ RUN      ] test_killblank_invalid
[       OK ] test_killblank_invalid
[ RUN      ] test_killblank_multiple
[       OK ] test_killblank_multiple
[ RUN      ] test_killblank_long
[       OK ] test_killblank_long
[ RUN      ] test_padding_top_bottom
[       OK ] test_padding_top_bottom
[ RUN      ] test_padding_invalid
[       OK ] test_padding_invalid
[ RUN      ] test_padding_negative
[       OK ] test_padding_negative
[ RUN      ] test_padding_notset
[       OK ] test_padding_notset
[ RUN      ] test_padding_invalid_value
[       OK ] test_padding_invalid_value
[ RUN      ] test_padding_novalue
[       OK ] test_padding_novalue
[ RUN      ] test_tabstops_zero
[       OK ] test_tabstops_zero
[ RUN      ] test_tabstops_500
[       OK ] test_tabstops_500
[ RUN      ] test_tabstops_4X
[       OK ] test_tabstops_4X
[ RUN      ] test_tabstops_4e
[       OK ] test_tabstops_4e
[ RUN      ] test_tabstops_4ex
[       OK ] test_tabstops_4ex
[ RUN      ] test_tabstops_7
[       OK ] test_tabstops_7
[ RUN      ] test_alignment_invalid_hX
[       OK ] test_alignment_invalid_hX
[ RUN      ] test_alignment_invalid_vX
[       OK ] test_alignment_invalid_vX
[ RUN      ] test_alignment_invalid_jX
[       OK ] test_alignment_invalid_jX
[ RUN      ] test_alignment_notset
[       OK ] test_alignment_notset
[ RUN      ] test_alignment_incomplete
[       OK ] test_alignment_incomplete
[ RUN      ] test_inputfiles_illegal_third_file
[       OK ] test_inputfiles_illegal_third_file
[ RUN      ] test_inputfiles_stdin_stdout
[       OK ] test_inputfiles_stdin_stdout
[ RUN      ] test_inputfiles_stdin
[       OK ] test_inputfiles_stdin
[ RUN      ] test_inputfiles_input_nonexistent
[       OK ] test_inputfiles_input_nonexistent
[ RUN      ] test_inputfiles_actual_success
[       OK ] test_inputfiles_actual_success
[ RUN      ] test_command_line_design_empty
[       OK ] test_command_line_design_empty
[ RUN      ] test_help
[       OK ] test_help
[ RUN      ] test_version_requested
[       OK ] test_version_requested
[==========] cmdline_tests: 35 test(s) run.
[  PASSED  ] 35 test(s).
[==========] regulex_tests: Running 5 test(s).
[ RUN      ] test_compile_pattern_error
[       OK ] test_compile_pattern_error
[ RUN      ] test_compile_pattern_empty
[       OK ] test_compile_pattern_empty
[ RUN      ] test_regex_replace_invalid_utf
[       OK ] test_regex_replace_invalid_utf
[ RUN      ] test_regex_replace_buffer_resize
[       OK ] test_regex_replace_buffer_resize
[ RUN      ] test_regex_replace_error
[       OK ] test_regex_replace_error
[==========] regulex_tests: 5 test(s) run.
[  PASSED  ] 5 test(s).
[==========] tools_tests: Running 12 test(s).
[ RUN      ] test_strisyes_true
[       OK ] test_strisyes_true
[ RUN      ] test_strisyes_false
[       OK ] test_strisyes_false
[ RUN      ] test_strisno_true
[       OK ] test_strisno_true
[ RUN      ] test_strisno_false
[       OK ] test_strisno_false
[ RUN      ] test_my_strrspn_edge
[       OK ] test_my_strrspn_edge
[ RUN      ] test_my_strrspn
[       OK ] test_my_strrspn
[ RUN      ] test_is_csi_reset
[       OK ] test_is_csi_reset
[ RUN      ] test_is_ascii_id_valid
[       OK ] test_is_ascii_id_valid
[ RUN      ] test_is_ascii_id_invalid
[       OK ] test_is_ascii_id_invalid
[ RUN      ] test_is_ascii_id_strict_valid
[       OK ] test_is_ascii_id_strict_valid
[ RUN      ] test_is_ascii_id_strict_invalid
[       OK ] test_is_ascii_id_strict_invalid
[ RUN      ] test_repeat
[       OK ] test_repeat
[==========] tools_tests: 12 test(s) run.
[  PASSED  ] 12 test(s).
[==========] unicode_tests: Running 8 test(s).
[ RUN      ] test_to_utf32
[       OK ] test_to_utf32
[ RUN      ] test_is_blank
[       OK ] test_is_blank
[ RUN      ] test_is_allowed_in_sample
[       OK ] test_is_allowed_in_sample
[ RUN      ] test_is_allowed_in_shape
[       OK ] test_is_allowed_in_shape
[ RUN      ] test_is_allowed_in_filename
[       OK ] test_is_allowed_in_filename
[ RUN      ] test_is_allowed_in_kv_string
[       OK ] test_is_allowed_in_kv_string
[ RUN      ] test_u32_strnrstr
[       OK ] test_u32_strnrstr
[ RUN      ] test_u32_insert_space_at
[       OK ] test_u32_insert_space_at
[==========] unicode_tests: 8 test(s) run.
[  PASSED  ] 8 test(s).
[==========] bxstring_tests: Running 60 test(s).
[ RUN      ] test_ascii_simple
[       OK ] test_ascii_simple
[ RUN      ] test_ascii_illegalchar
[       OK ] test_ascii_illegalchar
[ RUN      ] test_ascii_null
[       OK ] test_ascii_null
[ RUN      ] test_ansi_unicode_book
[       OK ] test_ansi_unicode_book
[ RUN      ] test_ansi_unicode_space_kinds
[       OK ] test_ansi_unicode_space_kinds
[ RUN      ] test_ansi_unicode_chinese
[       OK ] test_ansi_unicode_chinese
[ RUN      ] test_ansi_unicode_empty
[       OK ] test_ansi_unicode_empty
[ RUN      ] test_ansi_unicode_blanks
[       OK ] test_ansi_unicode_blanks
[ RUN      ] test_ansi_unicode_invisible_only
[       OK ] test_ansi_unicode_invisible_only
[ RUN      ] test_ansi_unicode_illegalchar
[       OK ] test_ansi_unicode_illegalchar
[ RUN      ] test_ansi_unicode_tabs
[       OK ] test_ansi_unicode_tabs
[ RUN      ] test_ansi_unicode_broken_escapes
[       OK ] test_ansi_unicode_broken_escapes
[ RUN      ] test_ansi_unicode_null
[       OK ] test_ansi_unicode_null
[ RUN      ] test_ansi_unicode_tc183
[       OK ] test_ansi_unicode_tc183
[ RUN      ] test_bxs_new_empty_string
[       OK ] test_bxs_new_empty_string
[ RUN      ] test_bxs_is_blank
[       OK ] test_bxs_is_blank
[ RUN      ] test_bxs_strdup
[       OK ] test_bxs_strdup
[ RUN      ] test_bxs_cut_front
[       OK ] test_bxs_cut_front
[ RUN      ] test_bxs_cut_front_zero
[       OK ] test_bxs_cut_front_zero
[ RUN      ] test_bxs_first_char_ptr_errors
[       OK ] test_bxs_first_char_ptr_errors
[ RUN      ] test_bxs_last_char_ptr
[       OK ] test_bxs_last_char_ptr
[ RUN      ] test_bxs_unindent_ptr_null
[       OK ] test_bxs_unindent_ptr_null
[ RUN      ] test_bxs_trimdup_null
[       OK ] test_bxs_trimdup_null
[ RUN      ] test_bxs_trimdup_invalid_startidx
[       OK ] test_bxs_trimdup_invalid_startidx
[ RUN      ] test_bxs_trimdup_invalid_endidx
[       OK ] test_bxs_trimdup_invalid_endidx
[ RUN      ] test_bxs_trimdup_invalid_endidx2
[       OK ] test_bxs_trimdup_invalid_endidx2
[ RUN      ] test_bxs_trimdup_normal
[       OK ] test_bxs_trimdup_normal
[ RUN      ] test_bxs_trimdup_vanish
[       OK ] test_bxs_trimdup_vanish
[ RUN      ] test_bxs_trimdup_ansi
[       OK ] test_bxs_trimdup_ansi
[ RUN      ] test_bxs_trimdup_ansi_same
[       OK ] test_bxs_trimdup_ansi_same
[ RUN      ] test_bxs_substr_errors
[       OK ] test_bxs_substr_errors
[ RUN      ] test_bxs_strcat
[       OK ] test_bxs_strcat
[ RUN      ] test_bxs_strcat_empty
[       OK ] test_bxs_strcat_empty
[ RUN      ] test_bxs_strcat_empty2
[       OK ] test_bxs_strcat_empty2
[ RUN      ] test_bxs_strcat_empty3
[       OK ] test_bxs_strcat_empty3
[ RUN      ] test_bxs_strchr
[       OK ] test_bxs_strchr
[ RUN      ] test_bxs_strchr_empty
[       OK ] test_bxs_strchr_empty
[ RUN      ] test_bxs_strchr_cursor
[       OK ] test_bxs_strchr_cursor
[ RUN      ] test_bxs_trim
[       OK ] test_bxs_trim
[ RUN      ] test_bxs_trim_blanks
[       OK ] test_bxs_trim_blanks
[ RUN      ] test_bxs_trim_none
[       OK ] test_bxs_trim_none
[ RUN      ] test_bxs_ltrim2
[       OK ] test_bxs_ltrim2
[ RUN      ] test_bxs_ltrim5
[       OK ] test_bxs_ltrim5
[ RUN      ] test_bxs_ltrim_empty
[       OK ] test_bxs_ltrim_empty
[ RUN      ] test_bxs_ltrim_max
[       OK ] test_bxs_ltrim_max
[ RUN      ] test_bxs_rtrim
[       OK ] test_bxs_rtrim
[ RUN      ] test_bxs_rtrim_empty
[       OK ] test_bxs_rtrim_empty
[ RUN      ] test_bxs_prepend_spaces_null
[       OK ] test_bxs_prepend_spaces_null
[ RUN      ] test_bxs_append_spaces
[       OK ] test_bxs_append_spaces
[ RUN      ] test_bxs_to_output
[       OK ] test_bxs_to_output
[ RUN      ] test_bxs_is_empty_null
[       OK ] test_bxs_is_empty_null
[ RUN      ] test_bxs_is_visible_char
[       OK ] test_bxs_is_visible_char
[ RUN      ] test_bxs_filter_visible
[       OK ] test_bxs_filter_visible
[ RUN      ] test_bxs_filter_visible_none
[       OK ] test_bxs_filter_visible_none
[ RUN      ] test_bxs_strcmp
[       OK ] test_bxs_strcmp
[ RUN      ] test_bxs_valid_anywhere_error
[       OK ] test_bxs_valid_anywhere_error
[ RUN      ] test_bxs_valid_in_filename_error
[       OK ] test_bxs_valid_in_filename_error
[ RUN      ] test_bxs_free_null
[       OK ] test_bxs_free_null
[ RUN      ] test_bxs_concat
[       OK ] test_bxs_concat
[ RUN      ] test_bxs_concat_nullarg
[       OK ] test_bxs_concat_nullarg
[==========] bxstring_tests: 60 test(s) run.
[  PASSED  ] 60 test(s).
[==========] remove_tests: Running 31 test(s).
[ RUN      ] test_match_outer_shape_null
[       OK ] test_match_outer_shape_null
[ RUN      ] test_match_outer_shape_left_anchored
[       OK ] test_match_outer_shape_left_anchored
[ RUN      ] test_match_outer_shape_left_shiftable
[       OK ] test_match_outer_shape_left_shiftable
[ RUN      ] test_match_outer_shape_left_shortened
[       OK ] test_match_outer_shape_left_shortened
[ RUN      ] test_match_outer_shape_left_not_found
[       OK ] test_match_outer_shape_left_not_found
[ RUN      ] test_match_outer_shape_left_too_far_in
[       OK ] test_match_outer_shape_left_too_far_in
[ RUN      ] test_match_outer_shape_left_edge
[       OK ] test_match_outer_shape_left_edge
[ RUN      ] test_match_outer_shape_right_anchored
[       OK ] test_match_outer_shape_right_anchored
[ RUN      ] test_match_outer_shape_right_shiftable
[       OK ] test_match_outer_shape_right_shiftable
[ RUN      ] test_match_outer_shape_right_shortened
[       OK ] test_match_outer_shape_right_shortened
[ RUN      ] test_match_outer_shape_right_shortened2
[       OK ] test_match_outer_shape_right_shortened2
[ RUN      ] test_match_outer_shape_right_not_found
[       OK ] test_match_outer_shape_right_not_found
[ RUN      ] test_match_outer_shape_right_too_far_in
[       OK ] test_match_outer_shape_right_too_far_in
[ RUN      ] test_match_outer_shape_right_edge
[       OK ] test_match_outer_shape_right_edge
[ RUN      ] test_shorten_preferleft_allowall
[       OK ] test_shorten_preferleft_allowall
[ RUN      ] test_shorten_preferleft_allowright
[       OK ] test_shorten_preferleft_allowright
[ RUN      ] test_shorten_preferleft_allowleft
[       OK ] test_shorten_preferleft_allowleft
[ RUN      ] test_shorten_preferright_allowall
[       OK ] test_shorten_preferright_allowall
[ RUN      ] test_shorten_corner_cases
[       OK ] test_shorten_corner_cases
[ RUN      ] test_hmm_sunny_day
[       OK ] test_hmm_sunny_day
[ RUN      ] test_hmm_sunny_day_short
[       OK ] test_hmm_sunny_day_short
[ RUN      ] test_hmm_missing_elastic_nne
[       OK ] test_hmm_missing_elastic_nne
[ RUN      ] test_hmm_invalid_input
[       OK ] test_hmm_invalid_input
[ RUN      ] test_hmm_empty_shapes_success
[       OK ] test_hmm_empty_shapes_success
[ RUN      ] test_hmm_backtracking
[       OK ] test_hmm_backtracking
[ RUN      ] test_hmm_shiftable
[       OK ] test_hmm_shiftable
[ RUN      ] test_hmm_shortened
[       OK ] test_hmm_shortened
[ RUN      ] test_hmm_shortened_right_fail
[       OK ] test_hmm_shortened_right_fail
[ RUN      ] test_hmm_shortened_right
[       OK ] test_hmm_shortened_right
[ RUN      ] test_hmm_blank_shiftable
[       OK ] test_hmm_blank_shiftable
[ RUN      ] test_hmm_blank
[       OK ] test_hmm_blank
[==========] remove_tests: 31 test(s) run.
[  PASSED  ] 31 test(s).
make[1]: Leaving directory '/Users/local/src/boxes/utest'

chorpler commented 12 months ago

Incidentally, I haven't yet merged in @mathomp4's changes from this mathomp4:add-macos-ci branch -- I created my branch off of the color-unicode branch, since the whole reason I wanted to build from source was to get ANSI box drawing characters to work... which they do (although the terminal font has a big effect on how nice they end up looking):

$ echo "boxes is great!" | out/boxes -d ansi
┌─────────────────┐
│ boxes is great! │
└─────────────────┘
$ echo "boxes is great!" | out/boxes -d ansidouble
╔═════════════════╗
║ boxes is great! ║
╚═════════════════╝
$ echo "boxes is great!" | out/boxes -d ansirounded
╭─────────────────╮
│ boxes is great! │
╰─────────────────╯
$ echo "boxes is great!" | out/boxes -d ansiheavy
┏━━━━━━━━━━━━━━━━━┓
┃ boxes is great! ┃
┗━━━━━━━━━━━━━━━━━┛
$ echo "boxes is great!" | out/boxes -d ansidashed
┌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┐
┊ boxes is great! ┊
└╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┘
$

tsjensen commented 12 months ago

@chorpler That looks very promising. Thank you for sharing! We would much appreciate you contributing this.

It seems to me that your changes and @mathomp4's should work together nicely. Maybe you could just update this PR? I would then rebase the color-unicode branch on top of it, so we can already build it on Mac.

tsjensen commented 11 months ago

It might be that updating this PR is not so straightforward because its content is from another fork. So, please feel free to open another PR if that's easier for you. 🙂

mathomp4 commented 11 months ago

@chorpler I'm willing to do whatever you'd like to get this in. You can either fork my fork and make a PR to my branch. Or, since my changes are so small, if you make a PR with your changes, I can merge your branch into this branch and we can see if the macos CI works with our combined power!

mathomp4 commented 11 months ago

Had some "fun" and added a bit of ugly bash to work around test 111 on macOS. Also fixed up the coveralls since we now have two checks instead of one.

I also added an "ignore error" for the issue that will probably be fixed by the work of @chorpler . But I wanted to make sure the full workflow would work if we had everything working. 😄

chorpler commented 11 months ago

Last night I tried to figure out the best way to add my changes. My first attempt at cherry-picking commits to merge into @mathomp4's branch ended up pulling in some changes from the color-unicode branch (from the same files I'd made changes in) and rendered the whole thing completely unbuildable. :D I ended up creating a branch from the mathomp4:add-macos-ci branch and just redoing my changes on top of it. Which was good, because I realized that if I called the function pointer bx_fprintf instead of bx_fprintf_ptr, and renamed the original bx_fprintf function to bx_fprintf_original, I could drastically cut down on the number of changes made.

Then I checked that things would still build and run on Linux and Windows.

On Fedora 37, where I built the color-unicode branch yesterday, it immediately threw an error about not being able to find the yacc command. Which is very strange, because the root Makefile had the YACC ?= bison line. I still haven't checked that out yet; I was able to build and get the unit tests to pass by setting the environment variable YACC=bison before doing the make commands.
On Windows 10, the build process worked fine, and boxes.exe worked as expected. But I got a large number of failing unit tests when I did make win32.utest. This is a brand new MingGW installation, so it's quite possible I made a mistake setting it up somewhere that is causing this.

Anyway, I'll go ahead and create the pull request to merge my new branch into the existing mathomp4:add-macos-ci branch, and we'll go from there.

chorpler commented 11 months ago

I created a pull request here: https://github.com/mathomp4/boxes/pull/1

Hopefully that will allow this pull request to be updated also. If it doesn't work, we can create a new pull request.

mathomp4 commented 11 months ago

I've pulled in the branch from @chorpler into my branch.

Good news: macOS passes both white-box and black-box tests! 🎉

Bad news: Linux is now failing white-box with segfaults and other stuff 😞 : https://github.com/ascii-boxes/boxes/actions/runs/6997063497/job/19033607396

I even tried adding:

        env:
          LEX: flex
          YACC: bison

to the action in case I hit the Fedora issue @chorpler saw above, but it didn't change anything.

I'm a Fortran programmer by trade so I'm not great at debugging C. Hopefully you all can see what's happening!

chorpler commented 11 months ago

OK, yeah, I see what I did. While redoing my changes in the add-macos-ci-update branch, I forgot to change the name of the bx_fprintf() function to bx_fprintf_original() in tools.c and using bx_fprintf as the function pointer (in order to not have to modify all the files that were calling bx_printf()) I forgot to also update the name of the function to be wrapped in utest/Makefile. Not surprisingly, trying to wrap a function pointer with a real function broke it. :P Good example of why CI is better than testing by hand!

I've submitted a new pull request for your mathomp4:add-macos-ci branch, @mathomp4. It looks like the build and build-macos steps have finished without errors this time.

chorpler commented 11 months ago

It occurs to me that as an alternative to changing line in utest/Makefile from MOCKS = bx_fprintf to MOCKS = bx_fprintf_original, we could just remove the --wrap flag entirely from utest/Makefile's flags_unix section, since the function pointer is handling it now. In that case, we might not even need the separate flags_darwin section. Maybe something to consider for later after we check that the function pointer will also work on Windows?

mathomp4 commented 11 months ago

I merged in the fix from @chorpler and things do indeed look good. Thanks!

I definitely have no way of testing the Windows branch of this. Perhaps @tsjensen can?

I suppose we could also add a Windows CI test but that is waaay beyond me. :smile:

mathomp4 commented 11 months ago

Undrafted! 🎉

tsjensen commented 11 months ago

I would appreciate if you removed the --wrap completely, if it's not too much trouble. That way, it is less likely that someone later thinks we use it and adds a new mock ...

chorpler commented 11 months ago

Sounds good, I'll remove it. Thanks for the tip on Windows bison using --defines, I hit the same problem and was trying to figure out what option to use without a man page handy.

tsjensen commented 11 months ago

I definitely have no way of testing the Windows branch of this. Perhaps @tsjensen can?
I suppose we could also add a Windows CI test [...]

122

tsjensen commented 11 months ago

One more thing, a bit about cosmetics ... I am a big fan of a linear commit history in my projects (no merge commits). So, I would like to do some cleanup of the commit history on this branch before I merge it, also rebasing it onto the latest master and so on. (Content will not be changed, of course.)

I would offer to do that myself, but only if it's OK with you. 😊 If you like, I can also merge mathomp4/boxes#3 while I'm at it.

mathomp4 commented 11 months ago

One more thing, a bit about cosmetics ... I am a big fan of a linear commit history in my projects (no merge commits). So, I would like to do some cleanup of the commit history on this branch before I merge it, also rebasing it onto the latest master and so on. (Content will not be changed, of course.)

I would offer to do that myself, but only if it's OK with you. 😊 If you like, I can also merge mathomp4#3 while I'm at it.

Huh. I didn't see a new PR. Good job GitHub notifications. Let me do that real quick.

And then after that, it's up to you. But first, merge time...

mathomp4 commented 11 months ago

Okay. The PR from @chorpler is merged in.

tsjensen commented 11 months ago

Ok, now we have nice, linear commits. 😸

I also made another small change regarding the defaults for $(LEX) and $(YACC). It turns out the ?= operator assigns the value only when the variable is undefined, but not if it has a default value, as all the predefined variables do. So the whole thing broke on Linux (it seems we tested Mac and Windows 😏). Now one must override lex and yacc with BX_LEX and BX_YACC.

tsjensen commented 11 months ago

Merged. Many thanks for your work @mathomp4 and @chorpler! 🎉

ascii-boxes / boxes

Add macOS CI #120

121

122