[Bug 825557] Review Request: mingw-clucene - CLucene 2.3.3.4 built for MinGW

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--- Comment #5 from greg.hellings@xxxxxxxxx ---
(In reply to comment #4)
> (In reply to comment #3)
> > I have uploaded an updated spec and src.rpm file to
> > the same URLs as above if anyone else wants to take a look.
> 
> Please bump the release and upload a new .src.rpm every time you make a
> change. This makes it easier to compare changes between versions

Noted. I figured the bumps should only happen once an actual build was
released. My bad.

> 
> Some additional review comments:
> - The CMake argument -DDISABLE_MULTITHREADING=ON and
> -D_CL_HAVE_WIN32_THREADS=0 are used in both the MINGW32_CMAKE_ARGS and the
> MINGW64_CMAKE_ARGS. To reduce duplication, you might want to move these to
> the %mingw_cmake call

I still hope to fix the issue regarding threads on 32-bit architecture, as I
know the 32-bit version can be built with threading support. Hence why I want
to keep them separated until I can focus on that.

> - The %clean section can be removed completely

Will do.

> - Why are the files %{mingw32_libdir}/CLuceneConfig.cmake and
> %{mingw64_libdir}/CLuceneConfig.cmake dropped but not the files
> %{mingw32_libdir}/CLucene/CLuceneConfig.cmake and
> %{mingw64_libdir}/CLucene/CLuceneConfig.cmake. Shouldn't these two files be
> dropped as well?

The .cmake files are analagous to .pc files. Since they are supplied by the
CLucene team they should be maintained, but they shouldn't be placed in the
general %{mingw*_libdir}/ root.

> - The CMake arguments -DLUCENE_SYS_INCLUDES:PATH=%{mingw32_libdir} and
> -DLUCENE_SYS_INCLUDES:PATH=%{mingw64_libdir} shouldn't be needed for MinGW
> packages. It looks like the native Fedora CLucene package uses it for
> multilib support, but as Fedora MinGW doesn't use multilib this CMake
> argument shouldn't be needed for this package

I'll give it a go building without it and see how it works out.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]