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

musl libc support #298

Merged
merged 1 commit into from
Feb 1, 2018
Merged

musl libc support #298

merged 1 commit into from
Feb 1, 2018

Conversation

jnbr
Copy link
Contributor

@jnbr jnbr commented Jan 30, 2018

musl has no macro, so it can not be detected straigthforward.

HAVE_STRTOD_L tells that strdod_l() is available
HAVE_LOCALE_H tells that the header locale.h is available

musl has no macro, so it can not be detected straigthforward.

HAVE_STRTOD_L tells that strdod_l() is available
HAVE_LOCALE_H tells that the header locale.h is available
@jnbr jnbr mentioned this pull request Jan 30, 2018
@bmatherly
Copy link
Member

LGTM. But Dan is the expert on this subject. We should wait for him to review after he gets back.

Copy link
Member

@ddennedy ddennedy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a moment, I was on the fence if we should use defined for all of these new macro references. However, it is not necessary and compile against glibc gives no warnings. I can add the parenthesis myself.

@@ -30,7 +30,7 @@
#include <sys/param.h>
#endif

#if defined(__GLIBC__) && !defined(__APPLE__)
#if defined(__GLIBC__) && !defined(__APPLE__) || HAVE_LOCALE_H
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have parenthesis to clarify the order of logical operations. && is higher precedence than ||:
#if (defined(__GLIBC__) && !defined(__APPLE__)) || HAVE_LOCALE_H

@ddennedy ddennedy merged commit 5b2c9de into mltframework:master Feb 1, 2018
@ddennedy ddennedy added this to the v6.8.0 milestone Feb 5, 2018
alfredfo added a commit to alfredfo/mlt that referenced this pull request Jun 22, 2022
reverts: mltframework#298.
There is no gurantee that either HAVE_STRTOD_L or HAVE_LOCALE_H will be
defined at compile-time. Try for example building this project :)

The locale usage is now defined in POSIX and therefore we can now
assume it will be available on Linux, except for ancient systems.
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/locale.h.html

Another way of dealing with this would be to have a global mlt_config.h
where HAVE_LOCALE_H and HAVE_STRTOD_L are defined to values determined
when mlt is installed.
@alfredfo alfredfo mentioned this pull request Jun 22, 2022
ddennedy pushed a commit that referenced this pull request Jun 25, 2022
reverts: #298.
There is no gurantee that either HAVE_STRTOD_L or HAVE_LOCALE_H will be
defined at compile-time. Try for example building this project :)

The locale usage is now defined in POSIX and therefore we can now
assume it will be available on Linux, except for ancient systems.
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/locale.h.html

Another way of dealing with this would be to have a global mlt_config.h
where HAVE_LOCALE_H and HAVE_STRTOD_L are defined to values determined
when mlt is installed.
This pull request was closed.
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.

3 participants