[Bug 492164] Review Request: healpix - Hierarchical Equal Area isoLatitude Pixelization of a sphere

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=492164





--- Comment #9 from Jussi Lehtola <jussi.lehtola at iki.fi>  2009-04-04 04:25:27 EDT ---
(In reply to comment #8)
> (In reply to comment #7)
> > rpmlint output:
> > chealpix.x86_64: W: no-soname /usr/lib64/libchealpix.so
> > chealpix.x86_64: W: shared-lib-calls-exit /usr/lib64/libchealpix.so
> > exit at GLIBC_2.2.5
> > chealpix-devel.x86_64: W: no-documentation
> > healpix-fortran.x86_64: W: no-soname /usr/lib64/libgif.so
> > healpix-fortran.x86_64: W: no-soname /usr/lib64/libhealpix.so
> > healpix-fortran-devel.x86_64: W: no-documentation
> > 6 packages and 0 specfiles checked; 0 errors, 6 warnings.
> 
> I believe these are OK, right? I mean, we don't want to craft a SONAME, do we?
> We can still ask upstream though.

Probably yes, these are just warnings anyway.

> > - Did you send the shared library patch upstream? IIRC some package sponsors
> > have frowned on Fedora specific patches to build shared libraries.
> 
> Heh, well, no, I had a little motivation fixing utterly broken makefiles. I may
> send it upstream, but I'm quite sure it breaks something about the static
> library compilation, and in fact does it impossible to compile statically w/o
> modifying the code. I'm not sure they would accept this.

Yeah; I spent some time some months ago trying to figure out how to package
this, but very quickly found out that I had no idea of how to do it. Your
example with the C part helped me get the Fortran stuff working.

> > - Even though there is only one C header, it might be logical to put it into
> > the same place as the Fortran module files.
> 
> Well, yes, I had exactly the same thought. You see, I decided not to change
> location when there was not Fortran module. I believe it might be a bad
> decision as well. We would need to change each program that includes it to
> -I%{_includedir}/healpix it, the very same thing we do with cfitsio. I believe
> leaving it in its upstream-decided traditional location has its upsides as
> well.

Well, if upstream uses includedir, then I don't see any reason to change it.
It's just one file anyway.

Still, I'm not sure about how logical it is; to compile a Fortran binding you
still need to supply -I/usr/include/healpix. You should make a comment about
this to healpix-devel.

> I have no problem moving it if you insist on it though.
> 
> > - Maybe Fortran package should be named just 'healpix'.
> 
> You decide. I see F90 library is called "libhealpix" in contrast to C's
> "libchealpix", which makes me tend to agree. Depends on where would a typical
> user of that library expect it to find. Honestly, I'm not exactly that kind of
> person (and am, in fact, secretly expecting you to comaintain the package and
> take care of the Fortran bindings ;)
> 
> Shortly put -- you seem to know the library much better than me, so I'd prefer
> stuff like naming packages and shifting files around up to you.

Yes, in retrospect I think "healpix" is best for the name of the Fortran
package and healpix-devel for the Fortran development modules, since IIUC
Healpix started out as a purely Fortran package and got the C, IDL and Java
stuff later.

I don't use healpix myself (I'm in materials physics), but I have a few friends
who use it. I can comaintain the package with you.

> > MUST: In the vast majority of cases, devel packages must require the base
> > package using a fully versioned dependency. NEEDSFIX
> > - fortran-devel must require fortran since the libraries are in fortran.
> 
> Hah, this was your code, no? ;)
> Anyways, will fix.

Yes, it was :)

> New package (probably just fixing the MUST items) following shortly.  

OK. Also, it occurred to me that there is a check phase that should be enabled:

make c-test
and
make f90-test

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.