This package is a revival of the recently retired version. The build will not work but I think this is a place to provide guidance and suggestion.

> Source0:
> Source1:

404 not found


> %package blender
> Summary: Blender integration scripts
> Requires: %{name} = %{version}-%{release}

Since there are arch-specific builds in these packages:
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package


> The build will not work but I think this is a place to provide
> guidance and suggestion.

Uhm, the build will not work? Why submit a package for review that doesn't even try to build, because the top sourcedir isn't found?

  RPM build errors:
  + cd YafaRay-3.1.1
  /var/tmp/rpm-tmp.p0hraO: line 37: cd: YafaRay-3.1.1: No such file or directory

Obviously, you need to adjust the name of the top sourcedir, which is Core-3.1.1-beta and not the default %{name}-%{version}.

You haven't even mentioned anything in the %changelog about the upgrade from 0.0.1 to 3.1.1 Beta.


> %{_libdir}/%{yname}/*.so
> %{_libdir}/libyafaraycore.so

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging:UnownedDirectories

Also for both [sub]packages and the libs installed into global %{_libdir}:
https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries

(In reply to Michael Schwendt from comment #2)
> > Source0:
> > Source1:
> 
> 404 not found
Fixed

> 
> 
> > %package blender
> > Summary: Blender integration scripts
> > Requires: %{name} = %{version}-%{release}
> 
> Since there are arch-specific builds in these packages:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

Added to the spec

> 
> > The build will not work but I think this is a place to provide
> > guidance and suggestion.
> 
> Uhm, the build will not work? Why submit a package for review that doesn't
> even try to build, because the top sourcedir isn't found?
> 
>   RPM build errors:
>   + cd YafaRay-3.1.1
>   /var/tmp/rpm-tmp.p0hraO: line 37: cd: YafaRay-3.1.1: No such file or
> directory
> 
> Obviously, you need to adjust the name of the top sourcedir, which is
> Core-3.1.1-beta and not the default %{name}-%{version}.

The correct sourcedir is in place. However, I hit an issue with opencv which somehow failed due to some missing headers even though the right dependency is fulfilled. See https://koji.fedoraproject.org/koji/watchlogs?taskID=17253204 and the result https://koji.fedoraproject.org/koji/taskinfo?taskID=17253202

Suggestion welcome.

> You haven't even mentioned anything in the %changelog about the upgrade from
> 0.0.1 to 3.1.1 Beta.

My bad. I forgot mention upgrade from upstream. 


> 
> > %{_libdir}/%{yname}/*.so
> > %{_libdir}/libyafaraycore.so
> 
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#File_and_Directory_Ownership
> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
> 
> Also for both [sub]packages and the libs installed into global %{_libdir}:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
Fixed. A relic of the old packaging method.


Here is the updated files
Spec URL: https://luya.fedorapeople.org/packages/SPECS/YafaRay.spec
SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.1.1-0.2.beta.fc25.src.rpm

Here is the updated files with latest git snapshot

Spec URL: https://luya.fedorapeople.org/packages/SPECS/YafaRay.spec
SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.2.0-0.1.git20170131.fc25.src.rpm

I notice usptream added qt-gui support disabled by default.

There is libtiff missing in the build dependencies, build fails with:

Make Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:148 (message):
  Could NOT find TIFF (missing: TIFF_LIBRARY TIFF_INCLUDE_DIR)
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:388 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake/Modules/FindTIFF.cmake:82 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  CMakeLists.txt:146 (find_package)
-- Configuring incomplete, errors occurred!

If you use mock also to build locally, you can spot all missing dependencies easily.

The rest of the SPEC file looks ok, but packaging guidelines for the snapshots are slightly different:

https://fedoraproject.org/wiki/Packaging:Versioning#Snapshot_packages
https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Commit_Revision

I would use something like this (date+git) as proposed in the page since you have 2 different snapshots:

%global commit0 0ab187e78ef1f16f3fbb22422dc614e8abd6aaef
%global commit1 ac472bc511ffa5b2706a02102fd85fe06bb24265
%global shortcommit0 %(c=%{commit0}; echo ${c:0:7})
%global shortcommit1 %(c=%{commit1}; echo ${c:0:7})
%global date 20170131

Version:    3.2.0
Release:    0.1.%{date}git%{?dist}

And then these lines in the Source tags:

Source0:	https://github.com/%{name}/Core/archive/%{commit0}.tar.gz#/Core-%{shortcommit0}.tar.gz
Source1:	https://github.com/%{name}/Blender-Exporter/archive/%{commit1}.tar.gz#/Blender-Exporter-%{shortcommit1}.tar.gz

Once the sources are properly declared, you can just run "spectool -g YafaRay.spec" to download the tarballs, the "#" after the source url means that the package should be named as such. In the end, by running spectool -g you should get these 2 files in the folder:

Core-0ab187e.tar.gz
Blender-Exporter-ac472bc.tar.gz

And then run rpmdev-bumspec -c "<comment>" YafaRay.spec to put the correct version/releases in the %changelog as well.

Please also remove the wget comment in the SPEC file, it's confusing, only spectool should be used if the source url is valid.

Alternatively, you can also set the conditions to check dynamically for the source url to check snapshots and for the release tag, for example:

Release:    0.1.%{?shortcommit0:.%{date}git}%{?dist}
%{?shortcommit0:Source0:    https://github.com/%{name}/Core/archive/%{commit0}.tar.gz#/Core-%{shortcommit0}.tar.gz}
%{!?shortcommit0:Source0:    https://github.com/%{name}/Core/archive/v%{version}.tar.gz#/Core-%{version}.tar.gz}

Here is the updated files with latest git snapshot

Spec URL: https://luya.fedorapeople.org/packages/SPECS/YafaRay.spec
SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.2.0-0.2.git20170212.fc25.src.rpm

(In reply to Luya Tshimbalanga from comment #7)
> SRPM URL:
> https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.2.0-0.2.git20170212.
> fc25.src.rpm

Actual URL is:

https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.2.0-0.2.20170212git.fc25.src.rpm

Sorry, now that you implemented the conditionals for setting source urls with conditionals you might want to do the same for the release tag so it avoids the date and the "git" word if it's a snapshot:

%date 20170212

Release:    16%{?date:.%{date}git}%{?dist}

Other than that, regarding SPEC file cosmetics, please align the %descriptions to 80 columns where possible.

While building, I see lots of messages like this one:

-- Set runtime path of "/builddir/build/BUILDROOT/YafaRay-3.2.0-0.2.20170212git.fc25.x86_64/usr/lib/libyafaray_v3_core.so" to "$ORIGIN/:$ORIGIN/../:$ORIGIN/../lib/"

Runtime path is not allowed in Fedora, you might want to set one of these (I'm looking at the CMake files):

CMAKE_SKIP_INSTALL_RPATH:BOOL=NO
CMAKE_SKIP_RPATH:BOOL=NO

And messages like this one:

extracting debug info from /builddir/build/BUILDROOT/YafaRay-3.2.0-0.2.20170212git.fc25.x86_64/usr/bin/yafaray-xml
readelf: Error: the dynamic segment offset + size exceeds the size of the file

It seems there are problems when creating the debuginfo packages, maybe the build is already doing something to the binaries.

- Stripping
- Using different build flags
- Not building with debug symbols

Maybe you should enable verbose building, see if the correct C/CXXFLAGS are applied, and see if the debug symbols are being built. Then play with the CMake options (I see some DEBUG flags in CMakeLists.txt).

Also, the build stops with the following:

+ /usr/lib/rpm/brp-python-bytecompile /usr/bin/python 1
Compiling /builddir/build/BUILDROOT/YafaRay-3.2.0-0.2.20170212git.fc25.x86_64/usr/share/blender/2.78/scripts/addons/yafaray/ot/yafaray_presets.py ...
  File "/usr/share/blender/2.78/scripts/addons/yafaray/ot/yafaray_presets.py", line 341
    class Yafaray_Menu(StructRNA, _GenericUI, metaclass=RNAMeta):  # Yafaray's own Preset Menu drawing: search method for files changed
                                                       ^
SyntaxError: invalid syntax
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.07Lyfo (%install)

I've played a bit with CMake options, with these 3 options you can fix Debuginfo extraction, Compiler flags and Runtime paths:

    -DCMAKE_SKIP_RPATH:BOOL=true \
    -DDEBUG_BUILD=Debug \
    -DUSER_DBGFLAGS="%{optflags}" \

Still looking how to enable verbose building.

Sed commands do not work for me in the form you've written them. If I check the CMakeLists.txt file I don't see any change. I've changed them from what's written in the spec file to this:

sed -i -e 's|set(YAF_LIB_DIR lib)|set(YAF_LIB_DIR %{_lib})|g' CMakeLists.txt
sed -i -e 's|set(YAF_TARGET_TYPE ARCHIVE DESTINATION ${CMAKE_INSTALL_PREFIX}/lib RUNTIME)|\
    set(YAF_TARGET_TYPE ARCHIVE DESTINATION ${CMAKE_INSTALL_PREFIX}/%{_lib} RUNTIME)|g' CMakeLists.txt

This changes the sed parameters and changes the lib directory into which they are installed from /usr/lib to /usr/lib64.

Also the %files section needs to be changed, from:

%{_prefix}/lib/%{yname}-plugins
%{_prefix}/lib/%{yname}_v3_interface.py*
%{_prefix}/lib/*.so

To:

%{_libdir}/%{yname}-plugins
%{_libdir}/%{yname}_v3_interface.py*
%{_libdir}/*.so

Ok, you just need the following to get verbose building:

%make_build VERBOSE=1

I confirm the compiler flags are correct with the above settings as in comment #10.

Sorry, I made a typo in comment #10; these are the parameters (Debug/ON):

    -DCMAKE_SKIP_RPATH:BOOL=true \
    -DDEBUG_BUILD=ON \
    -DUSER_DBGFLAGS="%{optflags}" \

The install section should be like this:

%install
%make_install VERBOSE=1

# Let RPM pick docs in the file section
rm -fr %{buildroot}%{_docdir}/%{yname}

And then I had to comment out the copying of the Blender-Exporter files due to the python error above. With all these changes, the packages builds fine with compiler flags, debug symbols and correct file list. :)

As soon as you integrate all these changes and fix the python error I will check the rest (rpmlint, options that we might want to enable, etc.).

Forgot one thing, you require the following:

BuildRequires: libappstream-glib

For the AppData validation in %check.

In reply to Simone Caronni from comment #8)
> Sorry, now that you implemented the conditionals for setting source urls
> with conditionals you might want to do the same for the release tag so it
> avoids the date and the "git" word if it's a snapshot:
> 
> %date 20170212
> 
> Release:    16%{?date:.%{date}git}%{?dist}

Thanks for the tip.

Here is the updated files with all fixes

Spec URL: https://luya.fedorapeople.org/packages/SPECS/YafaRay.spec 
SRPMS URL: https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.2.0-0.3.20170212git.fc25.src.rpm

Presets from blender add-on is temporarily disabled resulting a successful build and I filed the issue upstream.

There are many things to fix here. You have to remove the line:

%{_libdir}/%{yname}_v3_interface.py*

from the files section to make it build.

YafaRay.src: W: spelling-error Summary(en_US) raytracing -> ray tracing, ray-tracing, retracing
YafaRay.src: W: spelling-error %description -l en_US raytracing -> ray tracing, ray-tracing, retracing
YafaRay.src: W: spelling-error %description -l en_US Raytracing -> Ray tracing, Ray-tracing, Retracing

Please adjust to one of the suggestions.

YafaRay.src: W: invalid-license LGPLv2.1

This should be: LGPLv2+

http://fedoraproject.org/wiki/Licensing:Main#Software_License_List

YafaRay.src:23: W: mixed-use-of-spaces-and-tabs (spaces: line 14, tab: line 23)

Please use spaces everywhere in the SPEC file.

YafaRay.x86_64: W: devel-file-in-non-devel-package /usr/include/yafaray/core_api/shader.h
YafaRay.x86_64: W: devel-file-in-non-devel-package /usr/include/yafaray/materials/blendmat.h
[...]

All the headers should be in a separate package.

YafaRay.x86_64: E: script-without-shebang /usr/share/licenses/YafaRay/LICENSES
YafaRay.x86_64: E: incorrect-fsf-address /usr/share/licenses/YafaRay/LICENSES
YafaRay.x86_64: W: spurious-executable-perm /usr/share/doc/YafaRay/AUTHORS
YafaRay.x86_64: W: spurious-executable-perm /usr/share/doc/YafaRay/README
YafaRay-blender.x86_64: W: spurious-executable-perm /usr/share/doc/YafaRay-blender/README

Make sure all the docs have sane permissions (644).

YafaRay-debuginfo.x86_64: W: invalid-license LGPLv2.1

You can omit the license here, as the blender subpackage depends on the main package, so the license is installed anyway.

YafaRay-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/Core-22f0ac8f5705416ce7f401c4c15c2d3eb80ad51b/src/yafraycore/imagehandler.cc
YafaRay-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/Core-22f0ac8f5705416ce7f401c4c15c2d3eb80ad51b/src/integrators/bidirpath.cc
YafaRay-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/Core-22f0ac8f5705416ce7f401c4c15c2d3eb80ad51b/include/cameras/orthographicCamera.h
YafaRay-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/Core-22f0ac8f5705416ce7f401c4c15c2d3eb80ad51b/include/utilities/curveUtils.h
[...]

All the source files should be 644, not executables.
You can fix with a couple of finds in the %prep section:

find . -name "*.h" -exec chmod 644 {} \;
find . -name "*.c" -exec chmod 644 {} \;
find . -name "*.cc" -exec chmod 644 {} \;

Ecc..

Please, the next time you post a package for review make sure that at least it builds and at least check through the main guidelines.

(In reply to Simone Caronni from comment #18)
> Please, the next time you post a package for review make sure that at least
> it builds and at least check through the main guidelines.

Noted.

Here is the updated files with all fixes including latest git snapshot from 20170217/

Spec url: https://luya.fedorapeople.org/packages/SPECS/YafaRay.spec
SRPMS url: https://luya.fedorapeople.org/packages/SRPMS/YafaRay-3.2.0-0.4.20170217git.fc25.src.rpm

Result from rpmlint:
$ rpmlint ~/rpmbuild/SRPMS/YafaRay-3.2.0-0.4.20170217git.fc25.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Scratch build result:
https://koji.fedoraproject.org/koji/taskinfo?taskID=17920742

I had to set exclusivearch for i686 and x86_64 because of the failure in other architectures as described by rawhide scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=17920427

and following https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Support

Everything is fixed, package approved!

Thank you Simone!

Added block for architecture that failed to build.