lupinix's scratch build of astrometry-0.65-1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12574737

Doesn't build @rawhide :( http://koji.fedoraproject.org/koji/taskinfo?taskID=12574742

Btw: Are you sure license is BSD with advertising, not normal 3 clause BSD?

mattia's scratch build of astrometry-0.65-1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12574968

License text shipped with sources is like this:
https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#BSDwithAdvertising

See LICENSE file in sources main directory.

For the build failure in rawhide: I will check, most probably I forgot to add some buildrequires... locally on a F23 build machine it builds fine.

...forgot what I said, I messed up with licenses. It's BSD NO advertising, you're right.

I confirm that building on F23 works, Rawhide fails... reported upstream https://groups.google.com/forum/#!topic/astrometry/aDCjhfMYhpE

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.66-1.fc23.src.rpm

* Sat Jan 16 2016 Mattia Verga <mattia.verga> - 0.66-1
- Correct license to BSD
- Upgrade to 0.66 fix build on Rawhide

mattia's scratch build of astrometry-0.66-1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12578072

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.66-2.fc23.src.rpm

* Sun Jan 17 2016 Mattia Verga <mattia.verga> - 0.66-2
- -devel and -static are not noarch

mattia's scratch build of astrometry-0.66-2.fc23.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12584799

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.66-3.fc23.src.rpm

%changelog
* Sun Jan 17 2016 Mattia Verga <mattia.verga> - 0.66-3
- Add some index files for basic functionality
- Config file should not be overwritten
- Remove static libraries from package

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.66-4.fc23.src.rpm

%changelog
* Sun Jan 24 2016 Mattia Verga <mattia.verga> - 0.66-4
- Specify NetPBM paths
- Add wcslib BR
- Correct license to add GPLv2+ for index files

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.67-1.fc23.src.rpm

%changelog
* Sun Mar 03 2016 Mattia Verga <mattia.verga> - 0.67-1
- Update to 0.67
- Use pkgconfig for BR

mattia's scratch build of astrometry-0.67-1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=13249652

The Summary doesn't match the description. Just say what the tools do in the Summary.

In the comment about sources, provide explicit instructions how to download the sources, e.g. the arguments to the script if any any.

Would it work with python3? Python3 is preferred. If the python bits are a module to be used externally, you should provide both python2 and python3 versions if both are supported. Also, you should use the %{python_provide} macro [https://fedoraproject.org/wiki/Packaging:Python#The_.25python_provide_macro].

Another thing to mention:  The mandatory %__global_ldflags are *NOT* properly applied during the build…

Since the package uses SWIG to generate the wrappers, you can easily provide a Python2 AND Python3 version, by passing the correct python-libs to link against.

Thanks for the tips.
I've asked upstream and found that python code is usable externally. I thought it wasn't and that's why I didn't packaged a separate python package.
However, astronomy isn't compatible with python3.

I need some more help about that (python guidelines are not clear to me): I think I should create a subpackage named "astronomy-python2" with a "Provides: python2-astronomy" line, right? The %python_provide macro usage explanation is not clear to me...

Björn, about the %__global_ldflags: aren't they supposed to be applied with the ARCH_FLAGS="%{optflags}" passed to make?

(In reply to Mattia Verga from comment #18)
> I need some more help about that (python guidelines are not clear to me): I
> think I should create a subpackage named "astronomy-python2" with a
> "Provides: python2-astronomy" line, right?
The guidelines are not very clear. But what you describes sounds like the current standard practice.

> Björn, about the %__global_ldflags: aren't they supposed to be applied with
> the ARCH_FLAGS="%{optflags}" passed to make?
The only thing that counts is whether the expected ldflags are passed on to gcc during build. If the logs indicate that this isn't the case, you must find a different way to pass them.

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.67-3.fc23.src.rpm

%changelog
* Thu Mar 17 2016 Mattia Verga <mattia.verga> - 0.67-3
- Apply mandatory ld flags

* Wed Mar 16 2016 Mattia Verga <mattia.verga> - 0.67-2
- Add python2 subpackage

Build fails in mock for me. Log: https://in.waw.pl/~zbyszek/fedora/astrometry-build-log

(In reply to Zbigniew Jędrzejewski-Szmek from comment #21)
> Build fails in mock for me. Log:
> https://in.waw.pl/~zbyszek/fedora/astrometry-build-log

Strange, I've succesfully built it in Copr:
https://copr-be.cloud.fedoraproject.org/results/mattia/Astronomy/fedora-rawhide-x86_64/00168982-astrometry/build.log.gz

Seems that ARCH_FLAGS are not the same. In Copr:
make SYSTEM_GSL=yes -j2 'ARCH_FLAGS=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic'

In your mock:
make SYSTEM_GSL=yes -j12 'ARCH_FLAGS=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables'

...sorry, I saw later that your mock was i686.
So the only change is 'make -j2' vs 'make -j12'

I also made a scratch build in koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=13377711. It failed on amd64 arch.

-Werror strikes again:
Testing swap qsort_r...
((gcc -Werror -o os-features-test-swap-qsort \
     -g -Wall -ffinite-math-only -fno-signaling-nans -pthread -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -O3 -fomit-frame-pointer -DNDEBUG -fpic -Winline -I../include -I../include/astrometry -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DAN_GIT_REVISION='"0.67"' -DAN_GIT_DATE='"Mon_Jan_25_11:20:00_2016_-0500"' -DAN_GIT_URL='"https://github.com/dstndstn/astrometry.net"' -I../util -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libdrm -I/usr/include/libpng16  -I/usr/include/libpng16     -DWCSLIB_EXISTS      -I../include -I../include/astrometry  -I/usr/include/wcslib    -I../include -I../include/astrometry  -I/usr/include/wcslib -I. -DTEST_SWAP_QSORT_R os-features-test.c -Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -g -Wall -ffinite-math-only -fno-signaling-nans -pthread -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -O3 -fomit-frame-pointer -DNDEBUG -fpic -Winline >> os-features.log && \
   ./os-features-test-swap-qsort >> os-features.log && \
   echo "#define NEED_SWAP_QSORT_R 0") \
|| echo "#define NEED_SWAP_QSORT_R 1") >> ../include/astrometry/os-features-config.h.tmp
os-features-test.c: In function 'main':
os-features-test.c:68:36: error: passing argument 4 of 'qsort_r' from incompatible pointer type [-Werror=incompatible-pointer-types]
     qsort_r(array, N, sizeof(int), &mythunk, sortfunc);
                                    ^
In file included from os-features-test.c:5:0:
/usr/include/stdlib.h:767:13: note: expected '__compar_d_fn_t {aka int (*)(const void *, const void *, void *)}' but argument is of type 'int *'
 extern void qsort_r (void *__base, size_t __nmemb, size_t __size,
             ^~~~~~~
cc1: all warnings being treated as errors



There's also a bunch of interesting warnings:
kdtree_internal.c: In function 'kdtree_build_2_fff':
kdtree_internal.c:2247:2: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
  kd->lr[0] = N - 1;
  ^~
kdtree_internal.c:2243:5: note: ...this 'if' clause, but it is not
     if (options & KD_BUILD_LINEAR_LR)
     ^~

Might be worth looking into.

The problem is that F25 seems to have set parallel building to 16 concurrent threads (make -j16) while F24 had 4 (make -j4) and this doesn't like to astrometry package.
In fact, in F24 astrometry builds fine:
http://koji.fedoraproject.org/koji/taskinfo?taskID=13383841

By forcing the -j4 flag it also builds in rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=13384253

Now, since the Guidelines say "Whenever possible, invocations of make should be done as make %{?_smp_mflags}", I think I can use that custom value instead of default macro to overcome the problem, right?

Yes, you can. There's probably a bug in the build system, but not every bug has to be fixed (especially when it's in the build system, so it doesn't affect users).

Oh, one more note:
> F25 seems to have set parallel building to 16 concurrent threads 

This depends on the builder, not on the version. It's possible that rawhide get's bigger builders instead of slower smaller builders. There was some talk of new arm64 machines, I wonder if those are those.

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.67-4.fc23.src.rpm

%changelog
* Sat Mar 19 2016 Mattia Verga <mattia.verga> - 0.67-4
- Limit parallel make at 4 processes to fix build on F25
- Remove odd symlink in sources
- Fix wrong FSF address

I removed an odd link in sources main directory because it breaks the license check part of fedora-review.
About licenses (and the wrong FSF address fix) I've asked upstream to fix source headers, because the project was relicensed under BSD, but licenses in source headers are a mess:
https://groups.google.com/forum/#!topic/astrometry/mCuyze3TOeM

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.67-5.fc23.src.rpm

* Sun Mar 20 2016 Mattia Verga <mattia.verga> - 0.67-5
- Remove patented catalog and pictures from sources

Ole Streicher <olebole> from Debian pointed out that a catalog in sources is not free (free only for personal use), so I removed that.
I also removed demo files because their licenses are unclear.

Spec URL: http://www.coolbits.it/fedora/astrometry.spec
SRPM URL: http://www.coolbits.it/fedora/astrometry-0.67-5.fc24.src.rpm

Sorry, bad link.

libwcs has Summary "An implementation of the FITS World Coordinate System standard". I think astrometry should use the similar spelling for consistency (and abbrevs are best avoided):
Summary: Get World Coordinate System coordinates from an image

astrometry.x86_64: W: spelling-error %description -l en_US convertion -> conversion

The main package requires the python subpackage, and the python subpackage requires the main package. This defeats the splitting out of the python subpackage a bit, but it's OK (for example if python3 support is added in the future it will be useful). So nothing to change here, just taking note.

OTOH, the data package is not required by any of the other subpackages. Do other subpackages work without the -data subpackge? I'd expect the main subpackage to Require -data.

Calls to ldconfig are missing [https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries].

File /usr/bin/tabmerge from install of astrometry-0.67-5.fc23.x86_64 conflicts with file from package perl-Text-RecordParser-1.6.5-3.fc23.noarch.
That's a sign of a bigger problem. Many binaries in astrometry have very generic names: tablist, tabmerge, tabsort, plotxy, imstat, imcopy, listhead, liststruc, modhead, pad-file, subtable. tabmerge has to be renamed, but I think that the other binaries with "generic" names should be renamed too. Would an "astrometry-" or "wcs-" prefix work?

For the user it usually does not matter that executables are written in Python. Maybe you should drop the ".py" suffixes on executables in astrometry-python2?

Is this one still alive? Links for SRPM and spec are no longer working.

Sorry, I'm not planning to continue with this one. If someone wants to take it over, the SRPM and spec are available on copr under Mattia/Astronomy (the domain coolbits.it is dead).

I will go ahead and follow the policy for a stalled package review by closing this bug.

Opened a new review based on your packaging effort, fixing some things like conflicting names and unversioned soname.