FASTK icon indicating copy to clipboard operation
FASTK copied to clipboard

Published 20 hours ago • verified publisher icon thegenemyers

Fix fwrite() error handling and other minor build fixes

Open jmarshall opened this issue 3 years ago • 5 comments

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 avatar May 11 '21 14:05 jmarshall

Hi John,

Yes, sorry, the fwrite check was blown (I made a pass modifying all writes, both stdio and system, to make sure they succeeded). Thanks I pushed the fix to the master.

I tend to let people develop their own .gitignore, hopefully that's OK.

The third set of changes around HTSLIB I did not understand. I'm not very good with OS level issues, could you explain what this does, and most importantly will it work universally?

Best,  Gene

On 5/11/21, 4:24 PM, John Marshall wrote:

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.

    You can view, comment on, or merge this pull request online at:

https://github.com/thegenemyers/FASTK/pull/11

    Commit Summary

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/thegenemyers/FASTK/pull/11, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUSINVZJGY446ZCVG4BSDTTNE42TANCNFSM44VU5JGQ.

thegenemyers avatar May 11 '21 15:05 thegenemyers

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.

jmarshall avatar May 11 '21 16:05 jmarshall

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

thegenemyers avatar Aug 16 '21 20:08 thegenemyers

Hello Gene

I am copying John Marshall, who perhaps you meant to send this message to. I am pretty sure that you can build htslib in a way that tests for the existence of all the dependencies and only tries to link them in when they are present. I would guess this uses ./configure. Are you doing that?

Richard

On 16 Aug 2021, at 21:08, Eugene W Myers Jr @.***> wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/thegenemyers/FASTK/pull/11#issuecomment-899785820, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2FXZTTIIRJJYOH4AXDKPLT5FV3PANCNFSM44VU5JGQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

richarddurbin avatar Aug 16 '21 20:08 richarddurbin

Hi Richard,

Thanks for forwarding.  Giulio has in the mean time solved his

problem -- it had something to do with Conda.

 Honestly, I hate systems where the builds are so complicated that

one can't easily figure it out. I'm sure your right about configuration options.

-- Gene

On 8/16/21, 10:48 PM, Richard Durbin wrote:

Hello Gene

I am copying John Marshall, who perhaps you meant to send this message to. I am pretty sure that you can build htslib in a way that tests for the existence of all the dependencies and only tries to link them in when they are present. I would guess this uses ./configure. Are you doing that?

Richard

On 16 Aug 2021, at 21:08, Eugene W Myers Jr @.***> wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/thegenemyers/FASTK/pull/11#issuecomment-899785820, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2FXZTTIIRJJYOH4AXDKPLT5FV3PANCNFSM44VU5JGQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/thegenemyers/FASTK/pull/11#issuecomment-899808414, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUSINQWXW6KJ6A2I7L7WZTT5F2TLANCNFSM44VU5JGQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

thegenemyers avatar Aug 17 '21 13:08 thegenemyers