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.

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

chore: update vendored liblbfgs from https://github.com/chokkan/liblbfgs #51

Merged
merged 4 commits into from
Nov 20, 2024

Conversation

szhorvat
Copy link
Contributor

@szhorvat szhorvat commented Nov 18, 2024

This attempts to update liblbfgs, and causes a crash in the test suite. Let's check if the crash is due to an issue that is present independently of the liblbfgs update. Funny thing is, it doesn't happen when compiling in Debug mode.

This is what I see on my machine:

$ test/test_hzeta
dyld[29868]: main executable has no entry point
fish: Job 1, 'test/test_hzeta' terminated by signal SIGABRT (Abort)

Occurs only with test_hzeta.

@szhorvat
Copy link
Contributor Author

This is because tests use internal functions.

@szhorvat szhorvat force-pushed the lbfgs-update branch 2 times, most recently from 8a504f6 to 5864102 Compare November 18, 2024 10:44
@szhorvat
Copy link
Contributor Author

Why are the tests not failing here when using a shared library build? Some tests use non-public functions that don't get exported from the shard lib.

@szhorvat
Copy link
Contributor Author

I see there's some hackery in the TEST_CASES_INTERNAL section. But now Windows tests are failing when they shouldn't.

@szhorvat szhorvat force-pushed the lbfgs-update branch 4 times, most recently from 94e6f0a to 109362d Compare November 18, 2024 11:13
@szhorvat szhorvat force-pushed the lbfgs-update branch 3 times, most recently from fb0582f to c70ad1a Compare November 18, 2024 11:53
@szhorvat
Copy link
Contributor Author

szhorvat commented Nov 18, 2024

Windows tests are failing

This is because the DLL is not in the PATH. My attempted fix fails because the generator expression $<TARGET_FILE_DIR:plfit> is not getting evaluated. I don't know why, I give up.

EDIT: This is fixed now. The reason was that the add_test(name command) signature doesn't evaluate generator expressions. We need the add_test(NAME name COMMAND command) signature. But then we also need to use the -C option of ctest.

I see there's some hackery in the TEST_CASES_INTERNAL section.

The main executable has no entry point error might relate to this, but again I'm stuck and I give up.

EDIT: This is not the reason.

@szhorvat
Copy link
Contributor Author

The main executable has no entry point error occurs only with -O2 or -O3, and only with Apple Clang. It doesn't happen with GCC or LLVM Clang. Maybe let's just ignore this.

@szhorvat szhorvat marked this pull request as ready for review November 18, 2024 14:35
@szhorvat
Copy link
Contributor Author

I think we should merge this as-is:

  • It updates liblbfgs
  • Cleans up test headers
  • Expands CI runs to shared builds
  • Makes sure that shared build tests pass on Windows

Quite a heterogeneous set of changes, I know.

@ntamas ntamas merged commit ffa9ea7 into ntamas:main Nov 20, 2024
6 checks passed
@ntamas
Copy link
Owner

ntamas commented Nov 20, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants