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

build/meson: force LZ4_DEBUG=0 for tests/freestanding #1524

Merged
merged 1 commit into from
Oct 6, 2024
Merged

build/meson: force LZ4_DEBUG=0 for tests/freestanding #1524

merged 1 commit into from
Oct 6, 2024

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Oct 6, 2024

Match the Makefile, which never sets LZ4_DEBUG for this test. If LZ4_DEBUG is non-zero, lib/lz4.c #includes <assert.h>, the musl version of which has a conflicting definition for __assert_fail.

Fixes build failure:

../../../tests/freestanding.c:173:6: error: conflicting types for '__assert_fail'; have 'void(const char *, const char *, unsigned int,  const char *)'
  173 | void __assert_fail(const char * assertion, const char * file, unsigned int line, const char * function) {
      |      ^~~~~~~~~~~~~
In file included from ../../../tests/../lib/lz4.c:270,
                 from ../../../tests/freestanding.c:49:
/usr/include/assert.h:19:16: note: previous declaration of '__assert_fail' with type 'void(const char *, const char *, int,  const char *)'
   19 | _Noreturn void __assert_fail (const char *, const char *, int, const char *);
      |                ^~~~~~~~~~~~~

Match the Makefile, which never sets LZ4_DEBUG for this test.  If
LZ4_DEBUG is non-zero, lib/lz4.c #includes <assert.h>, the musl version of
which has a conflicting definition for __assert_fail:

    https://www.openwall.com/lists/musl/2019/03/04/6

Fixes build failure:

    ../../../tests/freestanding.c:173:6: error: conflicting types for '__assert_fail'; have 'void(const char *, const char *, unsigned int,  const char *)'
      173 | void __assert_fail(const char * assertion, const char * file, unsigned int line, const char * function) {
          |      ^~~~~~~~~~~~~
    In file included from ../../../tests/../lib/lz4.c:270,
                     from ../../../tests/freestanding.c:49:
    /usr/include/assert.h:19:16: note: previous declaration of '__assert_fail' with type 'void(const char *, const char *, int,  const char *)'
       19 | _Noreturn void __assert_fail (const char *, const char *, int, const char *);
          |                ^~~~~~~~~~~~~
@Cyan4973
Copy link
Member

Cyan4973 commented Oct 6, 2024

Great debugging @bgilbert !

@Cyan4973
Copy link
Member

Cyan4973 commented Oct 6, 2024

Regarding tests:
our current CI runs the freestanding test with make, but not with meson.
and I was wondering if this is something that could be added.

@Cyan4973
Copy link
Member

Cyan4973 commented Oct 6, 2024

Rectification:
there is a test named freestanding which is run during meson CI test :

 4/17 datagen                        OK               0.02s
 5/17 freestanding                   OK               0.02s
 6/17 lz4-contentSize                OK               4.15s

I'm not sure what it does exactly, maybe it just compiles tests/freestanding.
Moreover, it was obviously working fine so far, therefore it did not underline the need for LZ4_DEBUG=0.
Though maybe triggering this specific issue requires musl too, which is not standard.

@bgilbert
Copy link
Contributor Author

bgilbert commented Oct 6, 2024

Rectification: there is a test named freestanding which is run during meson CI test :

[...]

I'm not sure what it does exactly, maybe it just compiles tests/freestanding.

Meson compiles it and runs it.

Moreover, it was obviously working fine so far, therefore it did not underline the need for LZ4_DEBUG=0. Though maybe triggering this specific issue requires musl too, which is not standard.

That's right, the problem only occurs with musl.

@Cyan4973 Cyan4973 merged commit c612292 into lz4:dev Oct 6, 2024
64 checks passed
@bgilbert bgilbert deleted the freestanding branch October 6, 2024 22:19
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