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

Fix configure and build errors on OS X #160

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

clemensg
Copy link

Hi,

On OS X 10.10 I can only find readv and writev but not preadv or pwritev, so I substituted preadv with lseek+readv if preadv is not available, and did the same for pwritev.
I also removed a seemingly redundant autoconf check for clock_gettime, which broke the OS X configuration. The same check already exists in the case statements for platforms, which need clock_gettime.

I tested my changes on OS X 10.10 (Yosemite) and Debian Wheezy (make check tests passed).
I hope I did not break the build on any other platform.

If there is a better way to fix the build errors on OS X, please comment and I will update the PR.
Thanks.

Regards,
Clemens

@clemensg
Copy link
Author

Any thoughts?

@PiotrSikora
Copy link

I'm wondering why preadv and pwritev are used in the first place? It looks that the code would be much simpler and more portable with pread and pwrite.

@clemensg clemensg force-pushed the fix_osx_build_errors branch from 06dae2c to 36c0652 Compare December 27, 2014 16:00
@clemensg
Copy link
Author

Yes, I think so too. I changed the PR!

@ioerror
Copy link
Owner

ioerror commented Jan 5, 2015

How does this impact other OS X builds? Could you please report on any other impacted platforms as well?

@bfontaine
Copy link

On OS X 10.11.2 (15C50) tlsdate 0.0.13 doesn’t build without this patch.

@vapier
Copy link
Contributor

vapier commented Dec 10, 2015

your change is not equivalent -- it's changing the file offset when you use lseek. please change the code to use pread/pwrite instead.

you should also split the commits up -- one to change the configure script and one to update the pread/pwrite logic

@clemensg
Copy link
Author

Hi, sorry for the delay, I totally forgot about this PR.

@vapier , you are right. I modified the patch to use pread and pwrite and split up the commit.

@ioerror , I can only test ArchLinux, OS X 10.10 and OS X 10.11 but nothing older than that.
Maybe someone else has an older OS X machine to test it with?

Fixes build errors on OS X and has better readability.

Signed-off-by: Clemens Gruber <[email protected]>
@clemensg clemensg force-pushed the fix_osx_build_errors branch from 36c0652 to de4282e Compare December 10, 2015 15:57
@clemensg clemensg changed the title Fix configure and build errors on OS X Yosemite Fix configure and build errors on OS X Dec 10, 2015
@ilovezfs
Copy link

@ioerror @clemensg ping on this PR

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.

6 participants