Spec URL: https://ol.fedorapeople.org/libnoise.spec SRPM URL: https://ol.fedorapeople.org/libnoise-1.0.0-6.fc43.1.src.rpm Description: libnoise is a portable C++ library that is used to generate coherent noise, a type of smoothly-changing noise. Fedora Account System Username: ol
This is a dependency for orca-slicer package that is reviewed in bug 2403680.
Copr build: https://copr.fedorainfracloud.org/coprs/build/9688749 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403986-libnoise/fedora-rawhide-x86_64/09688749-libnoise/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Modernised spec file and fixed missing gcc-c++ dependency. Now it builds for Rawhide in mock successfully. Spec URL: https://ol.fedorapeople.org/libnoise.spec SRPM URL: https://ol.fedorapeople.org/libnoise-1.0.0-7.fc44.src.rpm
Created attachment 2109764 [details] The .spec file difference from Copr build 9688749 to 9689007
Copr build: https://copr.fedorainfracloud.org/coprs/build/9689007 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403986-libnoise/fedora-rawhide-x86_64/09689007-libnoise/fedora-review/review.txt Found issues: - License file COPYING.txt is not marked as %license Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libnoise Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
A package with this name already exists because this review is not for creating a completely new package, but to revive a retired one. A new release is available with a %license tag used instead of %doc for COPYING.txt file. Spec URL: https://ol.fedorapeople.org/libnoise.spec SRPM URL: https://ol.fedorapeople.org/libnoise-1.0.0-8.fc44.src.rpm
Created attachment 2109765 [details] The .spec file difference from Copr build 9689007 to 9689013
Copr build: https://copr.fedorainfracloud.org/coprs/build/9689013 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403986-libnoise/fedora-rawhide-x86_64/09689013-libnoise/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libnoise Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Name: libnoise Version: 1.0.0 Release: 8%{?dist} Summary: A general-purpose library that generates three-dimensional coherent noise License: LGPL-2.1-or-later URL: http://libnoise.sourceforge.net/ Source0: http://download.sourceforge.net/libnoise/libnoisesrc-%{version}.zip Patch0: libnoise-make.patch > Change to Patch: ... BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) > Remove this line, not needed last decade or so BuildRequires: gcc-c++ BuildRequires: libtool BuildRequires: doxygen BuildRequires: dos2unix > Please sort these lines %description libnoise is a portable C++ library that is used to generate coherent noise, a type of smoothly-changing noise. libnoise can generate Perlin noise, ridged multifractal noise, and other types of coherent-noise. Coherent noise is often used by graphics programmers to generate natural-looking textures, planetary terrain, and other things. %package devel Summary: Development files for %{name} Group: Development/Libraries Requires: %{name} = %{version}-%{release} %description devel The %{name}-devel package contains libraries and header files for developing applications that use %{name}. %package doc Summary: Documentation for %{name} Group: Development/Libraries Requires: %{name}%{?_isa} = %{version}-%{release} %description doc The %{name}-doc package contains documentation for developing applications that use %{name}. %prep %setup -q -c -n noise # The contents of the upstream zip file are a file called COPYING.txt # and a directory called 'noise' with the source. We don't want to # pollute the buildroot, so everything goes in a subdirectory and we # cd into the noise directory to build and install. cd noise %patch -P 0 -p0 > All this can be reduced to: %autosetup -p0 -n noise -b0 -C dos2unix -k ../COPYING.txt doc/html/doxygen.css %build # The makefile seems somewhat broken. If 'make' is run in the root # directory first, libnoise.a isn't generated. make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_OPT_FLAGS" cd .. make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_OPT_FLAGS" > Problem is that Makefile don't work with paraellel, reduce %build to %build make -j1 CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_OPT_FLAGS" to fix the problem. %install rm -rf $RPM_BUILD_ROOT > Remove this legacy line sed -i 's/\r//' COPYING.txt > This should be in %prep (as shown above) cd noise # make install does not work. mkdir -p $RPM_BUILD_ROOT/%{_defaultdocdir}/noise > There should no be any need for this line cp -p doc/htmldata/*png doc/html cp -p doc/htmldata/*css doc/html > If really needed, such move such be done in %prep dos2unix -k doc/html/doxygen.css > Same, see above rm include/Makefile mkdir -p $RPM_BUILD_ROOT/%{_includedir}/noise/ cp -pR include/* $RPM_BUILD_ROOT/%{_includedir}/noise/ > Can be written as: install -d -m0755 $RPM_BUILD_ROOT/%{_includedir}/noise rm include/Makefile cp -a -T include/ $RPM_BUILD_ROOT/%{_includedir}/noise/ mkdir -p $RPM_BUILD_ROOT/%{_libdir} cp -p lib/libnoise.so.0.3 $RPM_BUILD_ROOT/%{_libdir} > convert to single line: install -D -m0755 src/libnoise.so.0.3 $RPM_BUILD_ROOT/%{_libdir}/libnoise.so.0.3 find $RPM_BUILD_ROOT -name '*.la' -print0 | xargs -0 rm -f > avoid pipe by find $RPM_BUILD_ROOT -name '*.la' -delete ln -sf libnoise.so.0.3 $RPM_BUILD_ROOT/%{_libdir}/libnoise.so.0.3.0 ln -sf libnoise.so.0.3 $RPM_BUILD_ROOT/%{_libdir}/libnoise.so ln -sf libnoise.so.0.3 $RPM_BUILD_ROOT/%{_libdir}/libnoise.so.0 %files %license COPYING.txt %{_libdir}/libnoise.so.* %files devel %{_includedir}/noise/ %{_libdir}/libnoise.so %files doc %doc noise/doc/html
Created attachment 2109840 [details] Updated spec file Spec file with some issues resolved attached
(In reply to Terje Rosten from comment #9) > Patch0: libnoise-make.patch > > Change to Patch: ... Done. > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} > -n) > > > Remove this line, not needed last decade or so Done. > BuildRequires: gcc-c++ > BuildRequires: libtool > BuildRequires: doxygen > BuildRequires: dos2unix > > > Please sort these lines They are already sorted by importance. First two are necessary for compilation. Last two are just for docs. > %prep > %setup -q -c -n noise > # The contents of the upstream zip file are a file called COPYING.txt > # and a directory called 'noise' with the source. We don't want to > # pollute the buildroot, so everything goes in a subdirectory and we > # cd into the noise directory to build and install. > > cd noise > %patch -P 0 -p0 > > > > All this can be reduced to: > > %autosetup -p0 -n noise -b0 -C > dos2unix -k ../COPYING.txt doc/html/doxygen.css No, it can't. Patch should be applied in noise subdirectory. Also, I didn't quite understand what you mean by "-b0" option (it's ignored by %autosetup) and by "-C" option as well. > > %build > > # The makefile seems somewhat broken. If 'make' is run in the root > # directory first, libnoise.a isn't generated. > > make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_OPT_FLAGS" > > cd .. > make %{?_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_OPT_FLAGS" > > > Problem is that Makefile don't work with paraellel, reduce %build to > > %build > make -j1 CFLAGS="$RPM_OPT_FLAGS" LDFLAGS="$RPM_OPT_FLAGS" > > to fix the problem. Why do you think that it doesn't work in parallel? > > > %install > rm -rf $RPM_BUILD_ROOT > > Remove this legacy line Done. > sed -i 's/\r//' COPYING.txt > > This should be in %prep (as shown above) Done. > cd noise > > # make install does not work. > > mkdir -p $RPM_BUILD_ROOT/%{_defaultdocdir}/noise > > There should no be any need for this line Done. > cp -p doc/htmldata/*png doc/html > cp -p doc/htmldata/*css doc/html > > > If really needed, such move such be done in %prep Done. > dos2unix -k doc/html/doxygen.css > > Same, see above Done. > rm include/Makefile > mkdir -p $RPM_BUILD_ROOT/%{_includedir}/noise/ > cp -pR include/* $RPM_BUILD_ROOT/%{_includedir}/noise/ > > > Can be written as: > > install -d -m0755 $RPM_BUILD_ROOT/%{_includedir}/noise Done. > rm include/Makefile I see no reason to change order of lines, so I didn't put it after install above. > cp -a -T include/ $RPM_BUILD_ROOT/%{_includedir}/noise/ It can be written this way, but I think that existing variant is more readable to me. I didn't know what "-T" option means, had to read manual. Also, I'm not comfortable with overwriting a directory that was just created by install: it may change its permissions in unexpected ways. I'd better just copy files into this directory instead to be safe. > mkdir -p $RPM_BUILD_ROOT/%{_libdir} > cp -p lib/libnoise.so.0.3 $RPM_BUILD_ROOT/%{_libdir} > > > convert to single line: > > install -D -m0755 src/libnoise.so.0.3 > $RPM_BUILD_ROOT/%{_libdir}/libnoise.so.0.3 Done. > find $RPM_BUILD_ROOT -name '*.la' -print0 | xargs -0 rm -f > > avoid pipe by > > find $RPM_BUILD_ROOT -name '*.la' -delete Done.
Updates release is here. Spec URL: https://ol.fedorapeople.org/libnoise.spec SRPM URL: https://ol.fedorapeople.org/libnoise-1.0.0-9.fc44.src.rpm
Created attachment 2109855 [details] The .spec file difference from Copr build 9689013 to 9691984
Copr build: https://copr.fedorainfracloud.org/coprs/build/9691984 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403986-libnoise/fedora-rawhide-x86_64/09691984-libnoise/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libnoise Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
> No, it can't. Patch should be applied in noise subdirectory. > Also, I didn't quite understand what you mean by "-b0" option (it's ignored by %autosetup) and by "-C" option as well. Well, koji build package just fine with my suggested changes: https://koji.fedoraproject.org/koji/taskinfo?taskID=138174665 > Why do you think that it doesn't work in parallel? Because building serial work, see koji link above. Parallel build often changes build order which confuses some make files.
(In reply to Terje Rosten from comment #15) > > No, it can't. Patch should be applied in noise subdirectory. > > Also, I didn't quite understand what you mean by "-b0" option (it's ignored by %autosetup) and by "-C" option as well. > > Well, koji build package just fine with my suggested changes: > > https://koji.fedoraproject.org/koji/taskinfo?taskID=138174665 And now look at the build.log and see what it does: Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.18xR5U + umask 022 + cd /builddir/build/BUILD/libnoise-1.0.0-build + cd /builddir/build/BUILD/libnoise-1.0.0-build + rm -rf noise + /usr/lib/rpm/rpmuncompress -x /builddir/build/SOURCES/libnoisesrc-1.0.0.zip + STATUS=0 + '[' 0 -ne 0 ']' + /usr/lib/rpm/rpmuncompress -x -C noise /builddir/build/SOURCES/libnoisesrc-1.0.0.zip + STATUS=0 + '[' 0 -ne 0 ']' + cd noise + /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w . + /usr/lib/rpm/rpmuncompress /builddir/build/SOURCES/libnoise-make.patch + /usr/bin/patch -p0 -s --fuzz=0 --no-backup-if-mismatch -f So, it extracts the zip archive *twice*: first, into build root directory (polluting it with COPYING.txt file) and second, into noise subdirectory (as was supposed to do originally). Then it patches the upper copy (the one that is polluted). Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.XflUtv + umask 022 + cd /builddir/build/BUILD/libnoise-1.0.0-build ... + cd noise + make -j1 ... Everything is built in polluted build root. This is not good.
(In reply to Terje Rosten from comment #15) > > Why do you think that it doesn't work in parallel? > > Because building serial work, see koji link above. I was not asking about whether single-threaded build works. Of course, it works. I was asking about why you think that parallel build does not work here. > Parallel build often changes build order which confuses some make files. But not this one. GNU Make is good in parallelising builds, and I don't even know how to make it work incorrectly. May be, making some rule actions write the same files, but it's definitely not a correct way to use make. This particular Makefile dos nothing that makes rule actions clash when run in parallel, it just makes object files, then links them into a shared library. I use parallel build, and it works, and it works much faster. So, I see no reason to force build to be single-threaded.
The problem might be the following in top level Makefile: all: doc src include lib clean: cleandoc cleansrc cleaninclude cleanlib install: installinclude installlib doc src include lib: $(MAKE) -C $@ lib: include There are no dependency between lib and src targets and make will run those in parallel, however clearly lib target depend on src target as lib target just copy files compiled files from src/ to lib/: VPATH=../src/ .PHONY: all clean all: libnoise.a libnoise.la libnoise.so.0.3 -cp $? . If you change the following line in top level Makefile from: lib: include to lib: include src lib target will run after src is done and parallel make should work fine.
(In reply to Terje Rosten from comment #18) > The problem might be the following in top level Makefile: There is a workaround implemented already: make in noise/src subdirectory is run before make in noise directory.
> %package devel > Requires: %{name} = %{version}-%{release} > %package doc > Requires: %{name}%{?_isa} = %{version}-%{release} These are backwards. The -devel package is the one, which strictly depends on the base back in an arch-specific way. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package The -doc package doesn't need such a dependency at all. It can be installed independently from the library and the -devel package. For example, as to skim over the documentation without having to install the buildtime/runtime files. The -doc subpackage could even be made .noarch if its contents are the same for all target architectures.
(In reply to Michael Schwendt from comment #20) > The -doc package doesn't need such a dependency at all. It can be installed > independently from the library and the -devel package. For example, as to > skim over the documentation without having to install the buildtime/runtime > files. The -doc subpackage could even be made .noarch if its contents are > the same for all target architectures. OK, I've removed dependency on the main package from doc subpackage. Spec URL: https://ol.fedorapeople.org/libnoise.spec SRPM URL: https://ol.fedorapeople.org/libnoise-1.0.0-10.fc44.src.rpm
Created attachment 2112235 [details] The .spec file difference from Copr build 9691984 to 9758638
Copr build: https://copr.fedorainfracloud.org/coprs/build/9758638 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2403986-libnoise/fedora-rawhide-x86_64/09758638-libnoise/fedora-review/review.txt Found issues: - A package with this name already exists. Please check https://src.fedoraproject.org/rpms/libnoise Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#_conflicting_package_names Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.