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

Issue on macOS #31

Closed
markand opened this issue Aug 31, 2020 · 2 comments
Closed

Issue on macOS #31

markand opened this issue Aug 31, 2020 · 2 comments

Comments

@markand
Copy link
Contributor

markand commented Aug 31, 2020

Hi,

There are some compatibility issues on macOS that prevent me for putting it in homebrew right now, but I've figured out some workarounds.

Before sending a proper pull request, I wanted to show the quick and dirty fix and see how we can improve it if of course you mind adding support for macOS.

diff --git a/src/t_editor.c b/src/t_editor.c
index 4524fa1..55d3263 100644
--- a/src/t_editor.c
+++ b/src/t_editor.c
@@ -97,8 +97,13 @@ t_edit(struct t_tune *tune)
 	if (stat(tmp, &after) != 0)
 		goto out;
 
+#if defined(__APPLE__)
+	int modified = (after.st_mtimespec.tv_sec  > before.st_mtimespec.tv_sec ||
+		        after.st_mtimespec.tv_nsec > before.st_mtimespec.tv_nsec);
+#else
 	int modified = (after.st_mtim.tv_sec  > before.st_mtim.tv_sec ||
 		        after.st_mtim.tv_nsec > before.st_mtim.tv_nsec);
+#endif
 
 	/* we perform the load iff the file has been modified by the edit
 	   process and that process exited with success */
diff --git a/src/t_ftoggvorbis.c b/src/t_ftoggvorbis.c
index b3ad6b3..7447cf5 100644
--- a/src/t_ftoggvorbis.c
+++ b/src/t_ftoggvorbis.c
@@ -15,6 +15,10 @@
 #include "t_config.h"
 #include "t_backend.h"
 
+#if defined(__APPLE__)
+#define eaccess access
+#endif
+
 
 static const char libid[] = "libvorbis";
 
diff --git a/src/t_toolkit.h b/src/t_toolkit.h
index ee6567f..566b49b 100644
--- a/src/t_toolkit.h
+++ b/src/t_toolkit.h
@@ -7,16 +7,7 @@
  *
  */
 
-#ifdef FLAC_CLASH_WORKAROUND
-/*
- * FLAC's assert header conflict with the standard. It has been fixed in libFLAC
- * 1.3 (by requiring prefixing) but not all OS / distro have updated yet (Ubuntu
- * 13.04 for example).
- */
-#include </usr/include/assert.h>
-#else
 #include <assert.h>
-#endif
 
 #include <ctype.h>
 #include <err.h>
@kaworu
Copy link
Owner

kaworu commented Aug 31, 2020

hey @markand,

thanks for this issue. At first glance your patch does two distinct things:

  1. remove the compat for old FLAC headers
  2. add support for OSX

Both are fine, two separate commits under the same PR would be best.

Now for OSX, I would rather have both the eaccess and the mtim stuff detected by CMake so that the compat goo goes into t_config.h and be generally available (i.e. not scoped to where they are currently used).

Finally, I think we could use a MacOSX build and tests from Travis-CI so that further patches won't break. Ideally, we could even migrate to github actions on the way.

@markand
Copy link
Contributor Author

markand commented Aug 31, 2020

Good idea for the CMake test regarding the compatibility issue, I'll do that!

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

No branches or pull requests

2 participants