ucscGenomeBrowser / kent

UCSC Genome Browser source tree. Stable branch: "beta".
http://genome.ucsc.edu/
Other
219 stars 89 forks source link

knetUdc.c does not compile #13

Closed tillea closed 4 years ago

tillea commented 6 years ago

Hi, I cloned the current git (91029f6760f1660c6e124bbe37048d2c6bed19d0) and tried to build the lib via

cd src/lib
make

Unfortunately this does not build successfully:

cc -O -g  -Wall -Wformat -Wimplicit -Wreturn-type -Wuninitialized -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_GNU_SOURCE -DMACHTYPE_x86_64   -Wall -Wformat -Wimplicit -Wreturn-type -   Wuninitialized -I../inc -I../../inc -I../../../inc -I../../../../inc -I../../../../../inc -I../htslib -I/include -I/usr/include/libpng16  -o knetUdc.o -c knetUdc.c
knetUdc.c: In function ‘kuOpen’:
knetUdc.c:25:3: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
 kf->udcf = udcf;
   ^~
knetUdc.c:26:57: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
 verbose(2, "kuOpen: returning %lu\n", (unsigned long)(kf->udcf));
                                                         ^~
knetUdc.c: In function ‘kuRead’:
knetUdc.c:40:59: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
 verbose(2, "udcRead(%lu, buf, %lld)\n", (unsigned long)(fp->udcf), (long long)len);
                                                           ^~
knetUdc.c:41:25: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
 return (off_t)udcRead(fp->udcf, buf, (int)len);
                         ^~
knetUdc.c: In function ‘kuSeek’:
knetUdc.c:53:29: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
     offset = off+ udcTell(fp->udcf);
                             ^~
knetUdc.c:56:54: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
 verbose(2, "udcSeek(%lu, %lld)\n", (unsigned long)(fp->udcf), offset);
                                                      ^~
knetUdc.c:57:11: error: ‘knetFile {aka struct knetFile_s}’ has no member named ‘udcf’
 udcSeek(fp->udcf, offset);
           ^~

Kind regards

     Andreas.
NullModel commented 6 years ago

Good Morning Andreas: I'm curious if the source will compile if you set CC in your shell:

export CC=gcc

before you run your make ?

tillea commented 6 years ago

Thanks for the quick help. I can confirm that this helps when I run this in the original source tree. That's a bit strange since I'm using gcc anyway. However, my final goal is to build a Debian package and when doing so I tried to use the Debian packaged htslib. So I did

s#-I../htslib#-I/usr/include/htslib#

and this change brought back the issue I've reported above. I dived into the header files and found the reason. Your code copy of htslib is from version 1.3 and it has:

$ grep udcf src/htslib/htslib/knetfile.h 
    struct udcFile *udcf;

This definition was removed in version 1.7 which is currently in Debian unstable (nor was it brought back to version we have put in Debian experimental). Do you consider to upgrade the htslib code copy to more recent versions and adapt your library code to this? Kind regards, Andreas.

NullModel commented 6 years ago

Good Morning Andreas: I'm puzzled by your first report that it can not compile. The cc command line you display will compile the file just fine since it is referring to the kent source tree version of the htslib with -I../htslib

I do see that if you change the reference from -I../htslib to -I/usr/include it will pick up the installed /usr/include/htslib/*.h file and then show the error.

I have a debian environment here with versions: ` Linux osboxes 4.9.0-6-amd64 #1 SMP Debian 4.9.82-1+deb9u3 (2018-03-02) x86_64 GNU/Linux

gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1)

GNU Make 4.1

libhts-dev:amd64/stretch 1.3.2-2 uptodate libhts1:amd64/stretch 1.3.2-2 uptodate `

And can not reproduce the situation you describe. Until we upgrade our htslib version, I would suggest the work-around situation of using the htslib code in the kent source tree.

--Hiram

tillea commented 6 years ago

Hi Hiram, you obviously are using Debian stable. That's definitely a sane decision for a production system. However, for the purpose of developing new code I'd rather recommend to follow latest upstream of third party libraries. Htslib is even at version 1.8 currently which we have packaged for Debian experimental. We did not yet pushed to Debian unstable since python-pysam is lagging behind and we would break other packages. To enable you testing with htslib 1.7 I'll do a backport to Stretch and will drop you a note here once it is available on debian-backports. Thanks for your kind cooperation, Andreas.

tillea commented 6 years ago

Hi Hiram, you obviously are using Debian stable. That's definitely a sane decision for a production system. However, for the purpose of developing new code I'd rather recommend to follow latest upstream of third party libraries. Htslib is even at version 1.8 currently which we have packaged for Debian experimental. We did not yet pushed to Debian unstable since python-pysam is lagging behind and we would break other packages. To enable you testing with htslib 1.7 I'll do a backport to Stretch and will drop you a note here once it is available on debian-backports. Thanks for your kind cooperation, Andreas.

tillea commented 6 years ago

Hi again. To reproduce the problem with htslib1.7 on your system you can follow the Instructions for Debian Backports and then do

apt-get -t stretch-backports install libhts-dev

Hope this helps reproducing the issue, Andreas.

tillea commented 6 years ago

My question is: Do you plan to migrate to a more recent htslib version than 1.3? As I tried to explain I need to avoid code copied in Debian and the stumbling stone to build knetUdc.c (may be others) is that a quite old htslib (1.3) is used (1.9 was released meanwhile). It would help if you could uncover your plan for the foreseable future. Kind regards, Andreas.

maximilianh commented 6 years ago

I don't think we have a plan to upgrade htslib unless we find a reason to do so. Also, we will never use the most current htslib version, as every upgrade means that we have to do quite a bit of testing of the new htslib. So we will always be behind, for the forseeable future.

maximilianh commented 6 years ago

Unrelated question while at it: may I ask for which Debian package you're building knetfile? Is this source package a requirement for some other package? Thanks!

tillea commented 6 years ago

On Wed, Sep 05, 2018 at 02:34:49PM +0000, Maximilian Haeussler wrote:

Unrelated question while at it: may I ask for which Debian package you're building knetfile? Is this source package a requirement for some other package? Thanks! Currently seqsero would profit from it but other targets are on my list.

maximilianh commented 6 years ago

I see. thanks! You previously told us that you cannot package our code, because it's not under an OSS license. seqsero requires isPcr, which is not even under a free license (as far as I know, feel free to correct me), it's only free for academics. Can you package code like this? (in the non-free repo maybe?)

tillea commented 6 years ago

On Wed, Sep 05, 2018 at 02:33:45PM +0000, Maximilian Haeussler wrote:

I don't think we have a plan to upgrade htslib unless we find a reason to do so. Also, we will never use the most current htslib version, as every upgrade means that we have to do quite a bit of testing of the new htslib. So we will always be behind, for the forseeable future. I admit htslib is famous for breaking its interface. On the other hand my experience with several Debian packages depending from it became better over the last versions. While I can understand that your motivation to follow the latest and greatest regularly is low using a more recent one seems like a good idea to me. Kind regards, Andreas.

maximilianh commented 6 years ago

We appreciate your opinion.

Did you notice that the htslib in our tree is patched? I am not sure, but I don't think these patches have been submitted (and it may not be totally obvious to submit them). This is another reason why we have an older htslib in the tree.

tillea commented 6 years ago

On Wed, Sep 05, 2018 at 03:21:00PM +0000, Maximilian Haeussler wrote:

I see. thanks! You previously told us that you cannot package our code, because it's not under an OSS license. Not really. Parts of your code have a free license and I try to build those free parts for the moment. You might like to check the debian/copyright file of my packaging attempt: https://salsa.debian.org/med-team/libucsc/blob/master/debian/copyright

seqsero requires isPcr, SeqSero suggests isPcr. It works without and thus I was able to package it.

which is not even under a free license (as far as I know, feel free to correct me), it's only free for academic. I intended to start with the freely licensed parts of the library. Free for academic is clearly non-free and might end up in Debian non-free (in case I or somebody else might be motivated to do the packaging work).

Can you package code like this? (in the non-free repo maybe?) It depends from the actual license. We had a longish discussion about blat which would be an interesting target but that license makes it non-distributable (would be off topic here). My intention to package the freely licensed code was to split up some free parts and link the non-free parts (and may be other code - as I said, I found other projects using that lib) against it.

Kind regards, Andreas.

maximilianh commented 6 years ago

So seqsero does not require our code and as such you don't need to package our source or htslib. OK, I see, so seqsero is not the reason why you package our libs.

Ah. I see that you isolated the parts of our code that is in the public domain (didn't know that we use the MIT license) Great to hear that!! Thanks!

blat which would be an interesting target but that license makes it non-distributable

Yes blat is probably best viewed as commercial software

tillea commented 6 years ago

On Wed, Sep 05, 2018 at 08:33:23AM -0700, Maximilian Haeussler wrote:

Did you notice that the htslib in our tree is patched? No. I'm not at the state to inspect the actual code. My packaging went as far as: Remove all non-free stuff and all code copies and see whether it builds. I contacted you once the build failed.

I am not sure, but I don't think these patches have been submitted (and it may not be totally obvious to submit them). This is another reason why we have an older htslib in the tree. OK, understand. Finally this would be no killing blocker for a package. However, for sure my advise would be to submit the patches upstream. I'd even go so far to include those patches into the Debian package provided that 1) they apply to the version inside Debian and 2) do not break the htslib ABI. But for sure that's only the second best solution compared to an integration into upstream htslib. I actually see my task as Debian developer as a communication bridge between projects like yours and htslib to settle with a common codebase for common profit.

Kind regards, Andreas.

tillea commented 6 years ago

On Wed, Sep 05, 2018 at 03:46:06PM +0000, Maximilian Haeussler wrote:

So seqsero does not require our code and as such you don't need to package our source or htslib. OK, I see, so seqsero is not the reason why you package our libs.
Well, it enhances SeqSero and thus this is actually the reason. :-) I admit I'm not very focused on this but since I recently stumbled upon seqsero and I currently have some spare time slices while traveling I took up this stalled effort again. I need to check for other references. I know there were other packages. If I stumble again about these I'll add these to this thread if you are interested but no resources to seek for it currently.

Ah. I see that you isolated the parts of our code that is in the public domain (didn't know that we use the MIT license) Great to hear that!! Thanks! Yes! I was also happy to see this. I have the silent hope that you might consider to use this license for other parts of your code as well. ;-)

blat which would be an interesting target but that license makes it non-distributable Yes blat is commercial software Another part of my duty as Debian maintainer is to convince upstream to prefer free licenses - but I would not really like to misuse this thread for it.

Kind regards, Andreas.