Skip to content

Add unistd.h include to hts.c now that it used R_OK#1143

Merged
jkbonfield merged 1 commit intosamtools:developfrom
andrewpatto:develop
Sep 24, 2020
Merged

Add unistd.h include to hts.c now that it used R_OK#1143
jkbonfield merged 1 commit intosamtools:developfrom
andrewpatto:develop

Conversation

@andrewpatto
Copy link
Copy Markdown
Contributor

Include unistd to guarantee access to R_OK definition.

On a mac (Xcode 11) - when compiling via rust-htslib - I get a compile error about missing R_OK. As far as I can tell R_OK is defined officially in unistd.h, but probably comes in via other includes.

htslib by itself compiles fine on my mac - so I realise this is not specifically a htslib problem - presumably papered over a bit by the autoconf system.

The only other spot in htslib that used R_OK does include unistd (cram/cram_io.c)

Include unistd to guarantee acccess to R_OK definition
@andrewpatto andrewpatto changed the title Update hts.c Add unistd.h include to hts.c now that it used R_OK Sep 24, 2020
@andrewpatto
Copy link
Copy Markdown
Contributor Author

I should add that the the rust-htslib compile bound to the previous release of htslib did compile so this is something introduced in the latest release

@jkbonfield
Copy link
Copy Markdown
Contributor

jkbonfield commented Sep 24, 2020

On linux it looks to come via zlib.h -> zconf.h -> unistd.h.

Yes, we should be including it explicitly. Thanks. (Edit: I don't understand how it works by itself, but not via rust-htslib, but I assume that's something else changed such as a different zlib. Anyway the point is valid.)

@jkbonfield jkbonfield merged commit 3c1ea37 into samtools:develop Sep 24, 2020
@jmarshall
Copy link
Copy Markdown
Member

jmarshall commented Sep 24, 2020

POSIX does indeed specify that R_OK comes from unistd.h, and hts.c itself directly including what it itself needs is definitely the way to go.

It happens to come the same way on macOS; probably when compiling via Rust a different subset of the platform predefines that control whether zconf.h activates its #include <unistd.h> are defined (or perhaps the OP is on Catalina and there are differences nearby in the macOS 10.15 SDK), leading to that not being activated.

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