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

Use plain mkstemp() if O_NOATIME is not available #6

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

jmarshall
Copy link
Contributor

Platforms that don't have the Linux-specific O_NOATIME may not have the non-POSIX mkostemp() either.
For example, macOS prior to version 10.12 did not provide mkostemp().

The code needs mkostemp only so that it can specify mkostemp(…, O_NOATIME). If O_NOATIME is not available, it could equivalently use the POSIX standard mkstemp() instead. For your consideration, this PR does that.

See also bioconda/bioconda-recipes#37162, which applies this patch to Bioconda's unifrac-binaries package to reinstate it after previous macOS build failures due to this missing function.

@wasade
Copy link
Member

wasade commented Sep 27, 2022

Thank you! Will merge if tests pass

@jmarshall
Copy link
Contributor Author

Thanks! Unfortunately:

WARNING: The tools required to build C++ code for R were not found.

However this Ubuntu test failure appears to be unrelated to this code change…

@wasade
Copy link
Member

wasade commented Sep 27, 2022

Good spot, thanks. @sfiligoi, any idea what may be going on with the failure here?

@sfiligoi
Copy link
Collaborator

Looks like the dependency changed a little bit with never versions of gcc in conda.
Working on fixing the build script.

@wasade
Copy link
Member

wasade commented Sep 28, 2022

Thanks!

@sfiligoi
Copy link
Collaborator

@jmarshall Please pull the latests main. The build works again there.

Platforms that don't have O_NOATIME (i.e., non-Linux) may not
have mkostemp() either.
@sfiligoi sfiligoi merged commit 539e313 into biocore:main Oct 5, 2022
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