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.

Sign up for GitHub

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

Jump to bottom

Fix fwrite() error handling and other minor build fixes #11

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

Fix fwrite() error handling and other minor build fixes #11

wants to merge 3 commits into from

Conversation

jmarshall
Copy link

Prevent a warning by fixing the error handling on one of the fwrite() calls.

Also remove generated files in HTSLIB/* and add to Makefile so that it regenerates HTSLIB/htslib_static.mk if necessary. Ignore generated files so they don't clutter git status output.

jmarshall added 3 commits May 11, 2021 11:06
@jmarshall
Remove generated HTSlib files
3ad42aa 
Add HTSLIB/htslib_static.mk rule so that make will regenerate it
if it does not already exist. Change libhts.a rule to only build
what is needed, i.e., libhts.a.
@jmarshall
Ignore object files and other built files
fef0e5a
@jmarshall
Correct fwrite() error handling
352e76d
@jmarshall jmarshall mentioned this pull request May 11, 2021
Open
@thegenemyers
Copy link
Owner

thegenemyers commented May 11, 2021 via email

@jmarshall
Copy link
Author

I usually see platform-universal project-specific ignores in .gitignore, leaving .git/info/exclude available for people's local additions. But it's not a big deal.

Re the HTSLIB changes: the four files removed are generated by HTSLIB/Makefile and will have some differences on different people's machines. (e.g. there's a /Users/myersg/Desktop/… path in htslib-uninstalled.pc that will differ for everybody.) If someone needs to run htslib's configure as in #10, there will be significant differences.

So removing them from git is basically for the same reason as removing the object files from git: they're expected to differ for other people.

The files are regenerated by the HTSLIB/Makefile as needed anyway, but the one that is included by the top-level Makefile does need to be regenerated sooner so that it can be included by Make. Adding the rule enables that: GNU Make will remake that file immediately so that it can be included if it doesn't already exist. I believe this remaking of makefiles is specific to GNU Make, but the LIBDEFLATE makefile is already very GNU Make-specific anyway. So this does not further restrict universality.

The addition of make libhts.a to the libhts.a rule just speeds up the build a bit, avoiding building libhts.so, bgzip, tabix, and the test suite programs — as FASTK doesn't use them anyway.

@thegenemyers
Copy link
Owner

Hi John,
I'm at Rockefeller and my colleague Giulio Formenti is trying to install FastK on their cluster.
It fails as it can't find "bzlib.h" while in the midst of building htslib.a.
Is there any way to circumvent this? Cram wants a huge laundry list of compressors, and I'd imagine
often these are not all installed on a given machine. I thought that a change Richard Durbin had made
to my Makefile had fixed this [[$(HTSLIB_static_LIBS) on the gcc line]] but I don't know where this file
is or how it is made by the library make. I did incorporate all the changes you suggested with this pull
but it still doesn't work. Any help would be greatly appreciated.
Best, Gene

@richarddurbin
Copy link
Collaborator

richarddurbin commented Aug 16, 2021 via email

@thegenemyers
Copy link
Owner

thegenemyers commented Aug 17, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmarshall @thegenemyers @richarddurbin