https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Bug ID: 1366881 Summary: Review Request: ispc - C-based SPMD programming language compiler Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: luya@fedoraproject.org QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-1.fc24.src.rpm Description: A compiler for a variant of the C programming language, with extensions for "single program, multiple data" (SPMD) programming Fedora Account System Username: luya
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Luya Tshimbalanga luya@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1364618
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1364618 [Bug 1364618] Review Request: embree - Collection of high-performance ray tracing kernels developed at Intel
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Igor Gnatenko ignatenko@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |ignatenko@redhat.com
--- Comment #1 from Igor Gnatenko ignatenko@redhat.com ---
# Actual source is https://codeload.github.com/ispc/ispc/tar.gz/v1.9.1 # Due to github structure, use that name below for compilation Source0: https://github.com/ispc/ispc/archive/%%7Bname%7D-%%7Bversion%7D.tar.gz
Source0: https://github.com/ispc/ispc/archive/v%%7Bversion%7D/%%7Bname%7D-%%7Bversion...
install -m 755 -d %{buildroot}/%{_bindir} cp -p %{name} %{buildroot}/%{_bindir}
install -Dpm0755 %{name} %{buildroot}%{_bindir}/%{name}
BuildRequires: flex-devel https://codeload.github.com/ispc/ispc/tar.gz/v1.9.1
that something strange
BuildRequires: bison-devel
I don't think you really need devel subpkg of bison and flex, I think main pkg should be enough
* Missing BuildRequires: gcc-c++ * Missing BuildRequires: make * CFLAGS and LDFLAGS are not enforced -> I think %make_build OPT="%{optflags}" LDFLAGS="%{__global_ldflags}" should fix problem
I didn't check other stuff yet due to problems above.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #2 from Luya Tshimbalanga luya@fedoraproject.org --- (In reply to Igor Gnatenko from comment #1)
Source0: https://github.com/ispc/ispc/archive/v%%7Bversion%7D/%%7Bname%7D-%%7Bversion...
Thanks. That line is updated.
install -m 755 -d %{buildroot}/%{_bindir} cp -p %{name} %{buildroot}/%{_bindir}
install -Dpm0755 %{name} %{buildroot}%{_bindir}/%{name}
Fixed and noted.
BuildRequires: flex-devel https://codeload.github.com/ispc/ispc/tar.gz/v1.9.1
that something strange
Oops!That was an oversight where accidentally pressed middle button mouse. The url is now removed.
BuildRequires: bison-devel
I don't think you really need devel subpkg of bison and flex, I think main pkg should be enough
Devel subpkg replaced by their main packages.
- Missing BuildRequires: gcc-c++
- Missing BuildRequires: make
- CFLAGS and LDFLAGS are not enforced
-> I think %make_build OPT="%{optflags}" LDFLAGS="%{__global_ldflags}" should fix problem
%{optflag} part failed to build so I only use LDFLAGS. Missing BuildRequires gcc-c++ and make added.
Here is the updated Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-2.fc24.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #3 from Igor Gnatenko ignatenko@redhat.com ---
%{buildroot}/%{_bindir}/%{name}
%{buildroot}%{_bindir}/%{name}
BuildRequires: ncurses
there should be something wrong (or you need -devel subpkg of it or you most likely not need it)
BuildRequires: clang-devel
does it really require clang-devel for building?
* Still CFLAGS are ignored
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #4 from Luya Tshimbalanga luya@fedoraproject.org --- (In reply to Igor Gnatenko from comment #3)
%{buildroot}/%{_bindir}/%{name}
%{buildroot}%{_bindir}/%{name}
Fixed.
BuildRequires: ncurses
there should be something wrong (or you need -devel subpkg of it or you most likely not need it)
It turned out according to this scratch build http://koji.fedoraproject.org/koji/watchlogs?taskID=15256857 Added -devel subpkg
BuildRequires: clang-devel
does it really require clang-devel for building?
This scratch build suggested it otherwise it will fail http://koji.fedoraproject.org/koji/watchlogs?taskID=15256820
- Still CFLAGS are ignored
Added on the spec
I noticed koji will not properly build SRPM because of missing glibc-devel i686 which needs to be manually installed. I don't know how pull it within the spec file. http://koji.fedoraproject.org/koji/watchlogs?taskID=15256962 See https://github.com/ispc/ispc/wiki/Building-ispc:-Linux-and-Mac-OS-X
Here is the updated Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-3.fc24.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zbyszek@in.waw.pl
--- Comment #5 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl --- The trick is to require some file which is only provided by glibc-devel.i686. E.g. BuildRequires: /usr/lib/crt1.o.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #6 from Luya Tshimbalanga luya@fedoraproject.org --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
The trick is to require some file which is only provided by glibc-devel.i686. E.g. BuildRequires: /usr/lib/crt1.o.
Now koji complains about missing /usr/include/gnu/stubs.h:10:11: fatal error: 'gnu/stubs-64.h' file not found on x86 architectures
http://koji.fedoraproject.org/koji/taskinfo?taskID=15257891
Using %make_build CFLAGS="%{optflags}" LDFLAGS="%{__global_ldflags}" led to clang-3.8: warning: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-ld' /usr/bin/ld: cannot find -lz clang-3.8: error: linker command failed with exit code 1 (use -v to see invocation)
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #7 from Luya Tshimbalanga luya@fedoraproject.org --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
The trick is to require some file which is only provided by glibc-devel.i686. E.g. BuildRequires: /usr/lib/crt1.o.
Disregard comment #6, I figured out the failure. In combination of above, zlib-devel was also needed.
Using "LDFLAGS="%{__global_ldflags}"" above gave a warning message "clang-3.8: warning: argument unused during compilation: '-specs=/usr/lib/rpm/redhat/redhat-hardened-ld'". I left those parameters on the make_build line. http://koji.fedoraproject.org/koji/taskinfo?taskID=15261458
Only using CFLAGS seems to appease the complain from compilation by looking at the log. http://koji.fedoraproject.org/koji/taskinfo?taskID=15261384
Files are updated Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-3.fc24.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Luya Tshimbalanga luya@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ignatenko@redhat. | |com)
--- Comment #8 from Luya Tshimbalanga luya@fedoraproject.org --- Just following up as the updated spec and srpm are posted with recommended changes. See comment #7.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Igor Gnatenko ignatenko@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ignatenko@redhat. | |com) |
--- Comment #9 from Igor Gnatenko ignatenko@redhat.com --- * use gcc-c++ for building (add `gcc` into `%make_build` line) * replace CFLAGS with CXXFLAGS * it still ignores CXXFLAGS (Makefile needs patching) * verbose compilation is not enabled (I mean to show what and how has been executed). Remove `@` in lines like `@$(LEX)`, `@$(CXX)`, etc. * Trailing dot is missing in description
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #10 from Luya Tshimbalanga luya@fedoraproject.org --- Created attachment 1192486 --> https://bugzilla.redhat.com/attachment.cgi?id=1192486&action=edit patch form makefile
(In reply to Igor Gnatenko from comment #9)
- use gcc-c++ for building (add `gcc` into `%make_build` line)
Done
- replace CFLAGS with CXXFLAGS
Done, I add to declare -std=c++11 to remove the error.
- it still ignores CXXFLAGS (Makefile needs patching)
- verbose compilation is not enabled (I mean to show what and how has been
executed). Remove `@` in lines like `@$(LEX)`, `@$(CXX)`, etc.
Patched added. However it led to 3 errors.
I found a suggestion from llvm-config # Using http://llvm.org/docs/CommandGuide/llvm-config.html %make_build gcc 'llvm-config --cppflags --ldflags'
The failure occurred on those lines: t global scope: cc1plus: error: unrecognized command line option '-Wno-deprecated-register' [-Werror] cc1plus: error: unrecognized command line option '-Wno-c99-extensions' [-Werror] cc1plus: all warnings being treated as errors
- Trailing dot is missing in description
Added.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #11 from Luya Tshimbalanga luya@fedoraproject.org --- Files are updated Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-4.fc24.src.rpm
It seems there is a clash using gcc as builder who complain about some structural errors on module.cpp.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Luya Tshimbalanga luya@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ignatenko@redhat. | |com)
--- Comment #12 from Luya Tshimbalanga luya@fedoraproject.org --- So far g++ does not like this syntax on module.cpp:
module.cpp: In member function 'bool Module::writeOutput(Module::OutputType, const char*, const char*, DispatchHeaderInfo*)': module.cpp:1242:11: error: this 'if' clause does not guard... [-Werror=misleading-indentation] if (strcasecmp(suffix, "c") && strcasecmp(suffix, "cc") && ^~ module.cpp:1246:13: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' break; ^~~~~ module.cpp:1248:11: error: this 'if' clause does not guard... [-Werror=misleading-indentation] if (strcasecmp(suffix, "c") && strcasecmp(suffix, "cc") && ^~ module.cpp:1252:13: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if' break; ^~~~~ g++ will complain about these parameters:
cc1plus: error: unrecognized command line option '-Wno-deprecated-register' [-Werror] cc1plus: error: unrecognized command line option '-Wno-c99-extensions' [-Werror] cc1plus: all warnings being treated as errors
Which can be disabled with included patch.
clang as compiler does not complain about the issue. Should we keep using gcc or clang in that case? Upstream recommends the latter according to this link: https://github.com/ispc/ispc/wiki/Building-ispc:-Linux-and-Mac-OS-X
Perhaps making ispc available on the repo first should be the priority so you can provide the fixes by removing clang as requirement.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #13 from Zbigniew Jędrzejewski-Szmek zbyszek@in.waw.pl ---
cc1plus: all warnings being treated as errors
-Werror is a very bad idea. It can be useful in upstream development, where you are using a specific version of the compiler, and want to catch any warnings as they appear. But in a package in a distribution, new warnings will appear as the compiler is updated and becomes smarter, so -Werror will cause constant breakage. Just remove it.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #14 from Luya Tshimbalanga luya@fedoraproject.org --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #13)
cc1plus: all warnings being treated as errors
-Werror is a very bad idea. It can be useful in upstream development, where you are using a specific version of the compiler, and want to catch any warnings as they appear. But in a package in a distribution, new warnings will appear as the compiler is updated and becomes smarter, so -Werror will cause constant breakage. Just remove it.
Done via patch. The compilation no longer failed. It turned out CXXFLAGS and LDFLAGS were unnecessary on %make_build gcc lines as they were already in Makefile. Here is the updated files
Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-4.fc24.src.rpm
and the scratch build result http://koji.fedoraproject.org/koji/taskinfo?taskID=15347603
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #15 from Luya Tshimbalanga luya@fedoraproject.org --- Err... These are the right files Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-5.fc24.src.rpm
and the scratch build result http://koji.fedoraproject.org/koji/taskinfo?taskID=15347603
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Igor Gnatenko ignatenko@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ignatenko@redhat. | |com) |
--- Comment #16 from Igor Gnatenko ignatenko@redhat.com ---
g++ -O2 -I/usr/include -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG -I. -Iobjs/ -I/usr/include -DLLVM_3_8 -Wall -DBUILD_DATE=""20160823"" -DBUILD_VERSION="""no_version_info""" -Wno-sign-compare -Wno-unused-function -std=c++11 -fno-rtti -o objs/ast.o -c ast.cpp
as you can see all %{optflags} and %{__global_ldflags} are ignored.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #17 from Luya Tshimbalanga luya@fedoraproject.org --- Updated files with OPT and LDFLAGS added on %make_build gcc Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-5.fc24.src.rpm
Scratch build result http://koji.fedoraproject.org/koji/taskinfo?taskID=15351995
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #18 from Luya Tshimbalanga luya@fedoraproject.org --- ..was too quick to press button... Updated files with OPT and LDFLAGS added on %make_build gcc Spec URL: https://luya.fedorapeople.org/packages/SPECS/ispc.spec SRPM URL: https://luya.fedorapeople.org/packages/SRPMS/ispc-1.9.1-6.fc24.src.rpm
Scratch build result http://koji.fedoraproject.org/koji/taskinfo?taskID=15351995
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Igor Gnatenko ignatenko@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags| |fedora-review+
--- Comment #19 from Igor Gnatenko ignatenko@redhat.com --- Now it looks good ;)
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #20 from Luya Tshimbalanga luya@fedoraproject.org --- Thank you Igor!! =)
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #21 from Jon Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/ispc
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- ispc-1.9.1-7.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-01fee06612
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #23 from Fedora Update System updates@fedoraproject.org --- ispc-1.9.1-7.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-6ee40851f3
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- ispc-1.9.1-7.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-01fee06612
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #25 from Fedora Update System updates@fedoraproject.org --- ispc-1.9.1-7.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-6ee40851f3
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #26 from Fedora Update System updates@fedoraproject.org --- ispc-1.9.1-7.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2016-08-31 12:29:24
https://bugzilla.redhat.com/show_bug.cgi?id=1366881
--- Comment #27 from Fedora Update System updates@fedoraproject.org --- ispc-1.9.1-7.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org