Spec URL: https://raw.githubusercontent.com/SamStudio8/htslib-pkg/master/htslib.spec SRPM URL: https://github.com/SamStudio8/htslib-pkg/blob/master/htslib-1.3-1.fc23.src.rpm?raw=true Description: The current samtools package for Fedora is very out of date. The latest version of samtools relies on htslib, a C library for handling high-throughput sequencing data. However, this package is not available in the repository and is blocking an update to the samtools package. Please find my attempt at packaging htslib at the URL above. Fedora Account System Username: samstudio8
Hi Sam, There are some issues fedora-review found, which I'll come to, but perhaps most seriously there's a problem with the dependencies when I try to install it: Examining htslib-1.3-1.fc23.x86_64.rpm: htslib-1.3-1.fc23.x86_64 Marking htslib-1.3-1.fc23.x86_64.rpm to be installed Resolving Dependencies --> Running transaction check ---> Package htslib.x86_64 0:1.3-1.fc23 will be installed --> Processing Dependency: libhts.so.1()(64bit) for package: htslib-1.3-1.fc23.x86_64 --> Finished Dependency Resolution Error: Package: htslib-1.3-1.fc23.x86_64 (/htslib-1.3-1.fc23.x86_64) Requires: libhts.so.1()(64bit) Please see https://fedoraproject.org/wiki/PackagingDrafts/SharedLibraries for guidance on the packaging of shared libraries.
See also: - Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files directly in %_libdir. See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages - Static libraries in -static or -devel subpackage, providing -devel if present. Note: Package has .a files: htslib-devel. Does not provide -static: htslib-devel. See: http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries
Hi Adam, That does appear to be a rather serious problem, I've updated the SPEC and SRPM (available via the same URLs) in the hope that the package no longer effectively requires itself as a dependency. Sam
In the meantime, I looked at building this for an updated samtools and noticed these: Install LICENSE, and probably NEWS Add a proper %changelog Remove the explicit Provides Source URL is wrong Run "chmod 755" on the library for correct debuginfo packaging Run tests (in a %check section) Is there a good reason not to include bzip2 and/or lzma support? Maybe comment if so. DESTDIR=... is redundant with %make_install
Thanks for taking a look, Dave. Sam and I had been e-mailing privately about this and updating on Github, but it's better handled here. (In reply to Dave Love from comment #4) > In the meantime, I looked at building this for an updated samtools and > noticed these: > > Install LICENSE, and probably NEWS > > Add a proper %changelog > > Remove the explicit Provides > > Source URL is wrong > > Run "chmod 755" on the library for correct debuginfo packaging > > Run tests (in a %check section) > > Is there a good reason not to include bzip2 and/or lzma support? Maybe > comment if so. > > DESTDIR=... is redundant with %make_install
Thanks for your time, Dave. I've added LICENSE and NEWS to %doc, removed the unnecessary DESTDIR and Provides. I believe I have already solved the debuginfo issue with Adam's help off-list. Could you describe the issue with the source URL (https://github.com/samtools/htslib/releases/download/1.3.1/htslib-1.3.1.tar.bz2 works for me, for example). With regards to bzip2/lzma, I was under the impression the support is a little shaky at this time (see https://github.com/samtools/htslib/blob/develop/Makefile#L36-38) but support is optional, not required, for CRAM. I'll take a look at running tests in a %check block.
(In reply to Sam Nicholls from comment #6) > Could you describe the issue with the source URL > (https://github.com/samtools/htslib/releases/download/1.3.1/htslib-1.3.1.tar. > bz2 works for me, for example). I don't know what I saw at the tine, but now I see from rpmlint: htslib.spec: W: invalid-url Source0: https://github.com/samtools/htslib/releases/download/1.3.1/htslib-1.3.1.tar.bz2 HTTP Error 403: Forbidden I don't know why, since that URL does work with wget, and don't have time to debug it. > With regards to bzip2/lzma, I was under the impression the support is a > little shaky at this time (see > https://github.com/samtools/htslib/blob/develop/Makefile#L36-38) but support > is optional, not required, for CRAM. Fair enough, but worth a comment in the spec. The changelog is garbled. You should use rpmlint, or preferably fedora-review (which includes rpmlint) which the package has basically to pass anyhow.
(In reply to Dave Love from comment #7) > (In reply to Sam Nicholls from comment #6) > > Could you describe the issue with the source URL > > (https://github.com/samtools/htslib/releases/download/1.3.1/htslib-1.3.1.tar. > > bz2 works for me, for example). > > I don't know what I saw at the tine, but now I see from rpmlint: > > htslib.spec: W: invalid-url Source0: > https://github.com/samtools/htslib/releases/download/1.3.1/htslib-1.3.1.tar. > bz2 HTTP Error 403: Forbidden > > I don't know why, since that URL does work with wget, and don't have time to > debug it. I saw this with a different package. It's to do with the way Github handle encryption, which they said they're unlikely to change, when I reported it to them.
Hi Dave, With regards to the source URL, Adam's PR (https://github.com/SamStudio8/htslib-pkg/pull/1), notes that an issue with rpmlint has been reported upstream for invalid-url errors. Otherwise I'm stumped! I've added an appropriate comment and formatted the changelog appropriately (although I can't seem to shake the no-version-in-last-changelog).
(In reply to Sam Nicholls from comment #9) > Hi Dave, > With regards to the source URL, Adam's PR > (https://github.com/SamStudio8/htslib-pkg/pull/1), notes that an issue with > rpmlint has been reported upstream for invalid-url errors. Otherwise I'm > stumped! > > I've added an appropriate comment and formatted the changelog appropriately > (although I can't seem to shake the no-version-in-last-changelog). Yes, I was just about to post a link to the two relevant BZ reports: https://bugzilla.redhat.com/show_bug.cgi?id=1326855 https://bugzilla.redhat.com/show_bug.cgi?id=1328319
Hi Sam, Any news on this?
What is the current items to fix for below htslib.spec? https://github.com/SamStudio8/htslib-pkg/blob/master/htslib.spec Someone, could you summarize it? I can send a pull-request to SamStudio8/htslib-pkg repository, or I just can share my updated htslib.spec. As current htslib latest version is 1.9, I like updating it to 1.9. https://github.com/samtools/htslib
Hi I updated htslib.spec upgrading it to the latest version 1.9. Here is the pull-request. https://github.com/SamStudio8/htslib-pkg/pull/3 Here is the scratch build. https://koji.fedoraproject.org/koji/taskinfo?taskID=37504280 Below are the files on https://github.com/junaruga/htslib-pkg/tree/feature/update-to-1.9 . Spec URL: https://raw.githubusercontent.com/junaruga/htslib-pkg/feature/update-to-1.9/htslib.spec SRPM URL: https://github.com/junaruga/htslib-pkg/blob/feature/update-to-1.9/htslib-1.9-1.fc32.src.rpm?raw=true Could you review the pull-request or Spec/SRPM URLs? Thanks.
> Hi I updated htslib.spec upgrading it to the latest version 1.9. Here is the pull-request. > https://github.com/SamStudio8/htslib-pkg/pull/3 The pull-request was merged. Here are new URLs. Spec URL: https://raw.githubusercontent.com/SamStudio8/htslib-pkg/master/htslib.spec SRPM URL: https://github.com/SamStudio8/htslib-pkg/blob/master/htslib-1.9-1.fc32.src.rpm?raw=true Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37509125
Adam: can you still review this? From a quick look: licensecheck says Expat rather than MIT -- I haven't checked You could use %make_build in %build Remove the obsolete rm -rf from %install To support EPEL, use %ldconfig_scriptlets instead of %post... The library needs sorting out. Its soname is libhts.so.2, but it's installed as libhts.so.1.9 with a symlink to libhts.so.2. I haven't checked what's going on. The %files entry for it should be libhts.so.2*, so you know if it changes. If the version is actually taken from the release version -- I dont know -- it shouldn't be, and could be set to 0 for Fedora.
Dave - I could, but I'm rather out of practice. Seems like you're more up to date with the latest guidelines than I am...
Dave, Adam thanks for the reviewing. By the way, as I failed to run `fedora-review -b 1326504`, I reported opening the ticket to fedora-review: https://bugzilla.redhat.com/show_bug.cgi?id=1650620 . `fedora-review -n /path/to/htslib-1.9-1.fc32.src.rpm --rpm-spec` worked.
Created attachment 1614420 [details] review.txt by fedora-review command I attach review.txt file that is the result of the fedora-review command.
> By the way, as I failed to run `fedora-review -b 1326504`, I reported opening the ticket to fedora-review: https://bugzilla.redhat.com/show_bug.cgi?id=1650620 . Mistake. This URL. https://bugzilla.redhat.com/show_bug.cgi?id=1751630
Hi Dave, Here is the updated spec file and srpm file after your reviewing. Spec URL: https://raw.githubusercontent.com/junaruga/htslib-pkg/hotfix/review/htslib.spec SRPM URL: https://github.com/junaruga/htslib-pkg/blob/hotfix/review/htslib-1.9-1.fc32.src.rpm?raw=true Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37675570 A test to install binary RPMs: ok A test for rpmlint. There is below warning. But I like to postpone to fix it. ``` htslib.x86_64: W: shared-lib-calls-exit /usr/lib64/libhts.so.1.9 exit.5 This library package calls exit() or _exit(), probably in a non-fork() context. Doing so from a library is strongly discouraged - when a library function calls exit(), it prevents the calling program from handling the- error, reporting it to the user, closing files properly, and cleaning up any- state that the program has. It is preferred for the library to return an actual error code and let the calling program decide how to handle the- situation. ``` Below is the response for your review. > * licensecheck says Expat rather than MIT -- I haven't checked There is no "Expat" in the short name list used as "License:"'s value. https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses Also below package setting "MIT" is actually detected as "Expat" by licensecheck command. https://src.fedoraproject.org/rpms/golang-uber-zap/blob/master/f/golang-uber-zap.spec#_33 I think "MIT" is fine. > * You could use %make_build in %build done. > * Remove the obsolete rm -rf from %install done > * To support EPEL, use %ldconfig_scriptlets instead of %post... done. I found a good example for that. https://src.fedoraproject.org/rpms/libmodulemd/c/75a7af962341becaa6a7076e0b1e68c874acc7f9 > * The library needs sorting out. Its soname is libhts.so.2, but it's installed as libhts.so.1.9 with a symlink to libhts.so.2. I haven't checked what's going on. The %files entry for it should be libhts.so.2*, so you know if it changes. If the version is actually taken from the release version -- I dont know -- it shouldn't be, and could be set to 0 for Fedora. I checked the upstream's behavior. ``` $ git clone git:samtools/htslib.git $ cd htslib $ git checkout 1.9 $ make $ make install prefix=$(pwd)/dist ``` `make` command creates libhts.so (actual so file) and libhts.so.2 (symbolic link to libhts.so) on the current directory. But `make install prefix=$(pwd)/dist` creates below files soname: libhts.so.1.9 and symbolic links: libhts.so and libhts.so.2. libhts.so.1.9 is the actual soname used in the binary RPM file. ``` $ ls -l dist/lib/ total 10576 -rw-r--r-- 1 jaruga jaruga 7165054 Sep 15 21:54 libhts.a lrwxrwxrwx 1 jaruga jaruga 13 Sep 15 22:01 libhts.so -> libhts.so.1.9 -rw-r--r-- 1 jaruga jaruga 3653800 Sep 15 21:55 libhts.so.1.9 lrwxrwxrwx 1 jaruga jaruga 13 Sep 15 22:01 libhts.so.2 -> libhts.so.1.9 drwxr-xr-x 2 jaruga jaruga 4096 Sep 15 22:01 pkgconfig/ ``` Do you have any concerns about this situation? I suppose that current situation is no problem. And as an additional change, as Makefile "install" task includes "install-so" task, I removed "make "install-so" command line in htslib.spec. "make install" is good enough. Thanks!
(In reply to Adam Huffman from comment #16) > Dave - I could, but I'm rather out of practice. > Seems like you're more up to date with the latest guidelines than I am... I can have another look at the package and comment, but it is assigned to you, and I'd prefer more of a bioinformatics eye on it. I'm not in a good position to exercise such things.
> A test for rpmlint. There is below warning. But I like to postpone to fix it. > > ``` > htslib.x86_64: W: shared-lib-calls-exit /usr/lib64/libhts.so.1.9 I confess I ignore those. I'm not at all sure it's sensible as a general stipulation. > > * licensecheck says Expat rather than MIT -- I haven't checked > > There is no "Expat" in the short name list used as "License:"'s value. > https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses > > Also below package setting "MIT" is actually detected as "Expat" by > licensecheck command. > https://src.fedoraproject.org/rpms/golang-uber-zap/blob/master/f/golang-uber- > zap.spec#_33 > > I think "MIT" is fine. Ah. That probably merits a bug report. > `make` command creates libhts.so (actual so file) and libhts.so.2 (symbolic > link to libhts.so) on the current directory. > But `make install prefix=$(pwd)/dist` creates below files soname: > libhts.so.1.9 and symbolic links: libhts.so and libhts.so.2. libhts.so.1.9 > is the actual soname used in the binary RPM file. > > ``` > $ ls -l dist/lib/ > total 10576 > -rw-r--r-- 1 jaruga jaruga 7165054 Sep 15 21:54 libhts.a > lrwxrwxrwx 1 jaruga jaruga 13 Sep 15 22:01 libhts.so -> libhts.so.1.9 > -rw-r--r-- 1 jaruga jaruga 3653800 Sep 15 21:55 libhts.so.1.9 > lrwxrwxrwx 1 jaruga jaruga 13 Sep 15 22:01 libhts.so.2 -> libhts.so.1.9 > drwxr-xr-x 2 jaruga jaruga 4096 Sep 15 22:01 pkgconfig/ > ``` > > Do you have any concerns about this situation? > I suppose that current situation is no problem. It's simply wrong. .so.1.9 and .so.2 imply incompatible ABIs. Using 1.9 suggests that it's doing the wrong thing anyhow, by using the software version, not the ABI version, so you probably don't want to follow it. Without investigating, I don't know whether it would be best to use .so.1 or .so.2 in the soname. Then the spec should use that explicitly in %files, not a glob, to be proof against inadvertent changes. For packaging info see https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning Also, the advice is to glob man pages in %files <https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages>. If you use %make_build, it includes smp_mflags, so remove that. -fPIC is redundant in CFLAGS as it comes from the compiler specs. You should also set LDFLAGS to %build_ldflags or similar. Hope that helps.
Hi Dave, I updated the spec file and SRPM file for below URLs. > Spec URL: https://raw.githubusercontent.com/junaruga/htslib-pkg/hotfix/review/htslib.spec > SRPM URL: https://github.com/junaruga/htslib-pkg/blob/hotfix/review/htslib-1.9-1.fc32.src.rpm?raw=true > I confess I ignore those. I'm not at all sure it's sensible as a general stipulation. I just keep it in my mind at the moment. It's not an error from rpmlint, but warning. > Ah. That probably merits a bug report. Sure. I sent email to legal.org to ask the question about how to set Expat license. It looks Expat license is MIT license. Then someone replied Expat was same with MIT. https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/thread/C5AHVIW3F6LF5CYLR2PSHNANFYKP327P/ > It's simply wrong. .so.1.9 and .so.2 imply incompatible ABIs. ... > so you probably don't want to follow it. I simply did not understand it well. This is my first experience for RPM packaging of "foolib". I like to follow Fedora's rule as much as possible. I defined `%global so_version 0.1` to use it like libhts.so.0.1 I also opened the ticket to ask it on upstream. https://github.com/samtools/htslib/issues/932 > Also, the advice is to glob man pages in %files done > -fPIC is redundant in CFLAGS as it comes from the compiler specs. done > You should also set LDFLAGS to %build_ldflags or similar. done What I want to ask you is when I install the binary RPM htslib-1.9-1.fc32.x86_64.rpm, `/usr/lib64/libhts.so.2` is installed. I do not know why. ``` $ mock -i htslib-1.9-1.fc32.x86_64.rpm <mock-chroot> sh-5.0# ls /usr/lib64/libhts* /usr/lib64/libhts.so.0.1 /usr/lib64/libhts.so.2 <mock-chroot> sh-5.0# rpm -ql /usr/lib64/libhts.so.2 package /usr/lib64/libhts.so.2 is not installed ``` I ran `rm -f libhts.so.2` in %install section, also had to set `%exclude %{_libdir}/libhts.so.2` in %files. And I also below error from rpmlint might be related to this issue. ``` htslib.x86_64: E: no-ldconfig-symlink /usr/lib64/libhts.so.0.1 The package should not only include the shared library itself, but also the symbolic link which ldconfig would produce. (This is necessary, so that the link gets removed by rpm automatically when the package gets removed, even if for some reason ldconfig would not be run at package postinstall phase.) ``` Do you know why? Here is the scratch build. Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37811950 Thank you for your patience. I am learning a lot from this reviewing process.
Hi Dave, > I defined `%global so_version 0.1` to use it like libhts.so.0.1 > I also opened the ticket to ask it on upstream. > https://github.com/samtools/htslib/issues/932 I got a great feedback from a person in the upstream on the ticket. The htslib's so version rule is following Fedora's glibc's rule. So, can we use the built so files without changing so version, can't we? If you like, I will revert my modification for %make_build and %make_install line. ``` $ rpm -qf /usr/lib64/libc-2.29.so glibc-2.29-15.fc30.x86_64 $ rpm -qf /usr/lib64/libc.so.6 glibc-2.29-15.fc30.x86_64 $ rpm -qf /usr/lib64/libc.so glibc-devel-2.29-15.fc30.x86_64 $ ls -l /usr/lib64/libc[-.]* -rwxr-xr-x 1 root root 6699224 Jun 6 14:09 /usr/lib64/libc-2.29.so* -rw-r--r-- 1 root root 16258354 Jun 6 14:10 /usr/lib64/libc.a -rw-r--r-- 1 root root 253 Jun 6 13:55 /usr/lib64/libc.so lrwxrwxrwx 1 root root 12 Jun 6 13:55 /usr/lib64/libc.so.6 -> libc-2.29.so* ```
> Spec URL: https://raw.githubusercontent.com/junaruga/htslib-pkg/hotfix/review/htslib.spec > SRPM URL: https://github.com/junaruga/htslib-pkg/blob/hotfix/review/htslib-1.9-1.fc32.src.rpm?raw=true Hi I updated the htslib.spec and SRPM file. I renamed libhts-so.1.9 to libhts-1.9.so asking the question to the upstream here https://github.com/samtools/htslib/issues/932#issuecomment-536212327 to align glic(libc)'s style. ``` <mock-chroot> sh-5.0# ls -l /usr/lib64/libhts* -rwxr-xr-x 1 root root 759680 Sep 28 20:29 /usr/lib64/libhts-1.9.so lrwxrwxrwx 1 root root 13 Sep 28 20:29 /usr/lib64/libhts.so -> libhts-1.9.so lrwxrwxrwx 1 root root 13 Sep 28 20:29 /usr/lib64/libhts.so.2 -> libhts-1.9.so ``` On my local Fedora 30 environment. ``` $ ls -l /usr/lib64/libc[-.]* -rwxr-xr-x 1 root root 6699224 Jun 6 14:09 /usr/lib64/libc-2.29.so* -rw-r--r-- 1 root root 16258354 Jun 6 14:10 /usr/lib64/libc.a -rw-r--r-- 1 root root 253 Jun 6 13:55 /usr/lib64/libc.so lrwxrwxrwx 1 root root 12 Jun 6 13:55 /usr/lib64/libc.so.6 -> libc-2.29.so* ``` I think this situation is to solve the concern "It's simply wrong. .so.1.9 and .so.2 imply incompatible ABIs." * rpmlint: ok. * Test to install binary RPMs: ok * Scratch build: ok https://koji.fedoraproject.org/koji/taskinfo?taskID=37915859 Could you review again? Thanks.
Spec URL: https://raw.githubusercontent.com/junaruga/htslib-pkg/hotfix/review/htslib.spec SRPM URL: https://github.com/junaruga/htslib-pkg/blob/hotfix/review/htslib-1.9-1.fc32.src.rpm?raw=true I updated the spec and SRPM files again. > I renamed libhts-so.1.9 to libhts-1.9.so asking the question to the upstream here https://github.com/samtools/htslib/issues/932#issuecomment-536212327 to align glic(libc)'s style. I changed again. Now using upstream libraries directly without renaming. See https://github.com/samtools/htslib/issues/932#issuecomment-537694737 zlib on Fedora is same pattern with htslib. I think htslib is correct. We can update the Fedora guideine line later. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning * Koji scratch build: ok https://koji.fedoraproject.org/koji/taskinfo?taskID=38010659 * Test to install RPMs: ok * rpmlint: ok, as I mentioned we can fix below warning later, as this ticket blocks other tickets right now. ``` htslib.x86_64: W: shared-lib-calls-exit /usr/lib64/libhts.so.1.9 exit.5 This library package calls exit() or _exit(), probably in a non-fork() context. Doing so from a library is strongly discouraged - when a library function calls exit(), it prevents the calling program from handling the error, reporting it to the user, closing files properly, and cleaning up any state that the program has. It is preferred for the library to return an actual error code and let the calling program decide how to handle the situation. ``` As I assume that you are busy now, I will ask people to review on devel@ mailing list. :)
(In reply to Jun Aruga from comment #23) > What I want to ask you is when I install the binary RPM > htslib-1.9-1.fc32.x86_64.rpm, `/usr/lib64/libhts.so.2` is installed. I do > not know why. Nor do I, but it shouldn't be. > htslib.x86_64: E: no-ldconfig-symlink /usr/lib64/libhts.so.0.1 Don't take rpmlint too seriously. The link should be in the devel package.
(In reply to Jun Aruga from comment #24) > I got a great feedback from a person in the upstream on the ticket. > The htslib's so version rule is following Fedora's glibc's rule. glibc follows the normal rules about sonames, and has a policy to preserve backwards compatibility, so the major version is constant. > So, can we use the built so files without changing so version, can't we? > If you like, I will revert my modification for %make_build and %make_install > line. > > ``` > $ rpm -qf /usr/lib64/libc-2.29.so > glibc-2.29-15.fc30.x86_64 > > $ rpm -qf /usr/lib64/libc.so.6 > glibc-2.29-15.fc30.x86_64 > > $ rpm -qf /usr/lib64/libc.so > glibc-devel-2.29-15.fc30.x86_64 > > $ ls -l /usr/lib64/libc[-.]* > -rwxr-xr-x 1 root root 6699224 Jun 6 14:09 /usr/lib64/libc-2.29.so* > -rw-r--r-- 1 root root 16258354 Jun 6 14:10 /usr/lib64/libc.a > -rw-r--r-- 1 root root 253 Jun 6 13:55 /usr/lib64/libc.so > lrwxrwxrwx 1 root root 12 Jun 6 13:55 /usr/lib64/libc.so.6 -> > libc-2.29.so* The soname is libc.so.6, and there isn't a libc.so.<something else>. If you're not convinced, I should ask on the devel list, or at Red Hat, where there are libc maintainers.
> Nor do I, but it shouldn't be. Now I see not see the issue on my latest spec file. > If you're not convinced, I should ask on the devel list, or at Red Hat, where there are libc maintainers. Yes, please. Thank you. Have you read below conversation? There are 2 patterns both glib pattern and zlib pattern. https://github.com/samtools/htslib/issues/932 Could you send pull-request to junaruga/htslib-pkg hotfix/review branch as you like? Or just show me diff info? https://github.com/junaruga/htslib-pkg/tree/hotfix/review $ git clone git:junaruga/htslib-pkg.git $ cd htslib-pkg $ git checkout hotfix/review I am okay to follow your preference to finish the reviewing process. Because we can change spec file later if something needs to be changed after your clarification. As there are blocker tickets waiting this ticket, I want to finish it as soon as possible.
> Now I see not see the issue on my latest spec file. Sorry typo. "Now I do not see the issue on my latest spec file." is correct.
Sorry, it isn't my review to approve, and I can't spend much time on it. I'm happy to be corrected about shared libraries by someone authoritative in Fedora, and I don't know what htslib's policy is on ABI changes and whether the ELF semantics actually do follow the upstream version number.
Hi Dave, > Sorry, it isn't my review to approve, and I can't spend much time on it. I'm happy to be corrected about shared libraries by someone authoritative in Fedora, and I don't know what htslib's policy is on ABI changes and whether the ELF semantics actually do follow the upstream version number. Yeah. Alright. Please do "I'm happy to be corrected about shared libraries by someone authoritative in Fedora", and let me know. Hi Adam https://fedoraproject.org/wiki/Package_Review_Process#Review_Process > if you want to formally review the package, set the fedora-review flag to ? and assign the bug to yourself. I changed the flag to ? by following the the review process. I am "packager" in the document. I already inherited from original reporter Sam. [1] > ACCEPT - If the package is good, set the fedora-review flag to + I want to ask you to change "the fedora-review flag to +" from "?" if you are okay. [1] https://github.com/SamStudio8/htslib-pkg/pull/3#issuecomment-529875690
Adam, if you do not have a time to review, perhaps I can be the reviewer. In case, Sam is reporter, and I can change the fedora-review flag to "+".
(In reply to Jun Aruga from comment #23) > Hi Dave, > > I updated the spec file and SRPM file for below URLs. > > > Spec URL: https://raw.githubusercontent.com/junaruga/htslib-pkg/hotfix/review/htslib.spec > > SRPM URL: https://github.com/junaruga/htslib-pkg/blob/hotfix/review/htslib-1.9-1.fc32.src.rpm?raw=true > > > I confess I ignore those. I'm not at all sure it's sensible as a > general stipulation. > > I just keep it in my mind at the moment. It's not an error from rpmlint, but > warning. > > > Ah. That probably merits a bug report. > > Sure. I sent email to legal.org to ask the question > about how to set Expat license. It looks Expat license is MIT license. Then > someone replied Expat was same with MIT. > https://lists.fedoraproject.org/archives/list/legal@lists.fedoraproject.org/ > thread/C5AHVIW3F6LF5CYLR2PSHNANFYKP327P/ > > > It's simply wrong. .so.1.9 and .so.2 imply incompatible ABIs. ... > > > so you probably don't want to follow it. > > I simply did not understand it well. This is my first experience for RPM > packaging of "foolib". > I like to follow Fedora's rule as much as possible. > > I defined `%global so_version 0.1` to use it like libhts.so.0.1 > I also opened the ticket to ask it on upstream. > https://github.com/samtools/htslib/issues/932 > > > Also, the advice is to glob man pages in %files > > done > > > -fPIC is redundant in CFLAGS as it comes from the compiler specs. > > done > > > You should also set LDFLAGS to %build_ldflags or similar. > > done > > > What I want to ask you is when I install the binary RPM > htslib-1.9-1.fc32.x86_64.rpm, `/usr/lib64/libhts.so.2` is installed. I do > not know why. > > > ``` > $ mock -i htslib-1.9-1.fc32.x86_64.rpm > > <mock-chroot> sh-5.0# ls /usr/lib64/libhts* > /usr/lib64/libhts.so.0.1 /usr/lib64/libhts.so.2 > > <mock-chroot> sh-5.0# rpm -ql /usr/lib64/libhts.so.2 > package /usr/lib64/libhts.so.2 is not installed > ``` > > I ran `rm -f libhts.so.2` in %install section, also had to set `%exclude > %{_libdir}/libhts.so.2` in %files. > And I also below error from rpmlint might be related to this issue. > > ``` > htslib.x86_64: E: no-ldconfig-symlink /usr/lib64/libhts.so.0.1 > The package should not only include the shared library itself, but also the > symbolic link which ldconfig would produce. (This is necessary, so that the > link gets removed by rpm automatically when the package gets removed, even if > for some reason ldconfig would not be run at package postinstall phase.) > ``` > > Do you know why? > > Here is the scratch build. > Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=37811950 > > Thank you for your patience. > I am learning a lot from this reviewing process. Package approved. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "Expat License BSD 3-clause "New" or "Revised" License", "Expat License", "Public domain", "BSD 3-clause "New" or "Revised" License", "FSF All Permissive License". 107 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/htslib/review-htslib/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 30720 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: htslib-1.9-1.fc32.x86_64.rpm htslib-devel-1.9-1.fc32.x86_64.rpm htslib-tools-1.9-1.fc32.x86_64.rpm htslib-debuginfo-1.9-1.fc32.x86_64.rpm htslib-debugsource-1.9-1.fc32.x86_64.rpm htslib-1.9-1.fc32.src.rpm htslib.x86_64: W: spelling-error %description -l en_US samtools -> toadstools htslib.x86_64: W: spelling-error %description -l en_US bcftools -> footstools htslib.x86_64: W: shared-lib-calls-exit /usr/lib64/libhts.so.1.9 exit.5 htslib-tools.x86_64: W: spelling-error %description -l en_US tbi -> ti, bi, bit htslib-tools.x86_64: W: spelling-error %description -l en_US csi -> sci, cs, sic htslib.src: W: spelling-error %description -l en_US samtools -> toadstools htslib.src: W: spelling-error %description -l en_US bcftools -> footstools 6 packages and 0 specfiles checked; 0 errors, 7 warnings.
Hi Robert-André Mauchin Thanks for your review! Now I requested the repository and the branches by "fedpkg request-repo" and "fedpkg request-branch", and waiting they will be created. https://fedoraproject.org/wiki/Package_Review_Process#Review_Process $ fedpkg request-repo htslib 1326504 https://pagure.io/releng/fedora-scm-requests/issue/17763 $ fedpkg request-branch --repo htslib f31 https://pagure.io/releng/fedora-scm-requests/issue/17764 $ fedpkg request-branch --repo htslib f30 https://pagure.io/releng/fedora-scm-requests/issue/17765 $ fedpkg request-branch --repo htslib f29 https://pagure.io/releng/fedora-scm-requests/issue/17766
Above fedora-scm requests are declined, because I am not the creator this bugzilla ticket. I am asking Sam to request the repository and branches here. https://github.com/SamStudio8/htslib-pkg/pull/4
(In reply to Jun Aruga from comment #36) > Above fedora-scm requests are declined, because I am not the creator this > bugzilla ticket. > I am asking Sam to request the repository and branches here. > https://github.com/SamStudio8/htslib-pkg/pull/4 Yeah I was fearing this, alternatively create your own review request and close this bug as duplicate and block this bug as FE-DEADREVIEW. If Sam request the package, he will be the maintainer and not you.
> Yeah I was fearing this, alternatively create your own review request and close this bug as duplicate and block this bug as FE-DEADREVIEW. If Sam request the package, he will be the maintainer and not you. Thanks for sharing the alternative way. I am okay for that Sam will be maintainer. Then I can ask him for me to be a co-maintainer. I wait him today. tomorrow, if there is no response from him, I will do the alternative way.
Hi Jun, Thanks for your patch. I've requested the reviews as per your instructions: $ fedpkg request-branch --repo htslib f31 $ fedpkg request-repo htslib 1326504 https://pagure.io/releng/fedora-scm-requests/issue/17943 $ fedpkg request-branch --repo htslib f31 https://pagure.io/releng/fedora-scm-requests/issue/17944 $ fedpkg request-branch --repo htslib f30 https://pagure.io/releng/fedora-scm-requests/issue/17945 $ fedpkg request-branch --repo htslib f29 https://pagure.io/releng/fedora-scm-requests/issue/17946
I got: > The Bugzilla bug's review is approved by a user that is not a packager
Hi Sam, Sorry, I forget the possibility for you. You need to get a sponsor from other fedora users before requesting the repository. I can be a sponsor for you. Let me check the actual steps. > https://fedoraproject.org/wiki/Package_Review_Process#Review_Process > > If you do not have any package already in Fedora, this means you need a sponsor and to add FE-NEEDSPONSOR (Bugzilla id:177841) to the bugs being blocked by your review request. For more information read the How to get sponsored into the packager group wiki page.
I don't think this should have been waved through with the shared libraries in that state. It may or may not strictly be incorrect to link libhts.so.1.9 with soname libhts.so.2, but it's at least misleading when someone familiar with ELF versioning looks at a program's dynamic linkage or what's in libdir. Did someone on devel say that's OK?
> I don't think this should have been waved through with the shared libraries in that state. > It may or may not strictly be incorrect to link libhts.so.1.9 with soname libhts.so.2, but it's at least misleading when someone familiar with ELF versioning looks at a program's dynamic linkage or what's in libdir. Did someone on devel say that's OK? I do not say that you are wrong. I asked you to discuss on the htslib upstream, or to send a pull-request on comment 29. https://bugzilla.redhat.com/show_bug.cgi?id=1326504#c29 Because I am not familiar with the ELF, and I do not have an ability to fix the spec file aligning your requests. I thought that you declined the options because you did not have a time. If you act to make your idea real, the situation moves forward. But otherwise not. By the way, my account's role is just "user", not "sponsor". I can not be a sponsor of Sam at the moment. So, I want to ask someone to be a sponsor of Sam.
> https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Submitting_quality_new_packages > When you open your review request in bugzilla, you should block the FE-NEEDSPONSOR tracking bug, that way all of the sponsors will be able to see your sponsorship request. I blocked "FE-NEEDSPONSOR" following above document.
Dave, how you think about the fact of the libraries that is same pattern with upstream htslib? I do not think that your concern is valid considering the fact at the moment. ``` $ ls -l /usr/lib64/libz.so* lrwxrwxrwx 1 root root 14 Aug 15 09:30 /usr/lib64/libz.so -> libz.so.1.2.11* lrwxrwxrwx 1 root root 14 Aug 15 09:30 /usr/lib64/libz.so.1 -> libz.so.1.2.11* -rwxr-xr-x 1 root root 128904 Aug 15 09:30 /usr/lib64/libz.so.1.2.11* $ rpm -qf /usr/lib64/libz.so zlib-devel-1.2.11-17.fc30.x86_64 $ rpm -qf /usr/lib64/libz.so.1 zlib-1.2.11-17.fc30.x86_64 $ rpm -qf /usr/lib64/libz.so.1.2.11 zlib-1.2.11-17.fc30.x86_64 ``` Please join to discuss with us on following page, if you would like to change the reviewer's decision. https://github.com/samtools/htslib/issues/932#issuecomment-537694737
I am asking if current htslib is valid on devel@ mailing list. https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/WHZBVT3Y7TKYAEK6EXEY6L77W42T7LJX/
> Did someone on devel say that's OK? No. No response so far.
Jan: I think you should open a new review ticket and close this one as DEAD-REVIEW. Sam's review request from 3 years ago is not enough for be sponsored as a packager.
Jun and I have discussed this off-thread and think this is likely the best way forward.
I close following ticket, as I can request the dist-git repository by my account that has packager "user" role on Fedora. https://bugzilla.redhat.com/show_bug.cgi?id=1759940 *** This bug has been marked as a duplicate of bug 1759940 ***