https://bugzilla.redhat.com/show_bug.cgi?id=1132661 --- Comment #6 from Michael Schwendt <bugs.michael@xxxxxxx> --- > probably you need new version of gyp compiler. That's what should be worked on first: http://bugz.fedoraproject.org/gyp Figure out whether a new version is needed, then submit an update request ticket in bugzilla and make it block this review ticket. Becoming a co-maintainer of "gyp" would also be one of the ways to get sponsored: https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group [...] About this review request. Start with pointing the fedora-review tool at this ticket: fedora-review -b 1132661 If you follow the documentation for new packagers, the tool will evaluate the "Spec URL:" and "SRPM URL:" lines, do a test-build of the package and check many things. It isn't a full review, but very often finds issues. It also runs rpmlint on all packages, which is a MUST during review anyway > # Install gyp > svn co http://gyp.googlecode.com/svn/trunk gyp That's won't work. The Fedora Build System (http://koji.fedoraproject.org/koji/ and with Mock as build tool) does not permit accessing the network. > %package libs > Summary: Library to chromium > Group: Development/Libraries Runtime (!) libs have been in group "System Environment/Libraries" for many years. The Group tag is optional nowadays, however. https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag > Requires: %{name}-libs = %{version} https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > %description libs > Libraries need for atom This needs an explanation. Why does it need and include libudev.so.0 while Fedora's udev is at libudev.so.1 already. This is pretty much a blocker, not just because of the no-bundling policy: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries It also makes no sense to ship runtime libs in a -libs subpackage just for fun. If they are a strict requirement at runtime and there is no public API either, a subpackage is the wrong choice. Nothing else would need that subpackage. > Requires: nodejs > Requires: http-parser > Requires: zsh Uncommented explicit Requires are a major source of errors: https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires Too often, packagers either don't keep these requirements up-to-date, or the required packages don't really include want is needed (subpackages or changed versions and contents), or the software must be patched to fix paths. Add comment in spec files more often than not. > %install > #rm -rf $RPM_BUILD_ROOT > CFLAGS='%{optflags} -g -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' ; export CFLAGS > CXXFLAGS='%{optflags} -g -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' ; export CXXFLAGS Why do you need CFLAGS and CXXFLAGS in %install? Also, buildroot is deleted automatically nowadays. No need to keep the "#rm -rf $RPM_BUILD_ROOT" commented out, since %buildroot and $RPM_BUILD_ROOT should not be mixed anyway: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS > install -pm755 %{buildroot}%{_datadir}/atom/libchromiumcontent.so %{buildroot}%{_libdir} > install -pm755 %{buildroot}%{_datadir}/atom/libudev.so.0 %{buildroot}%{_libdir} rm -Rf /tmp/atom-build This is highly suspicious. Why are arch-specific libs installed into /usr/share/atom? How are they found and loaded at runtime? And why are they moved to %_libdir without a comment? At least the spec file ought to give a rationale here. > %{_datadir}/applications/Atom.desktop https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files If desktop-file-utils are used implicitly, please add a comment. > %post > > %preun > > %postun Delete unused scriptlet sections. They will cause you major pain, if you don't want any such scriptlets but include scripts by accident (e.g. by spec file comments in a false place below them). > %clean >#rm -rf $RPM_BUILD_ROOT https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean > %files > %defattr(-,root,root,-) Notice that it's the default %defattr nowadays, and you didn't add it for the -libs package either, so better get rid of it: https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > %dir %{_datadir}/atom > %{_datadir}/atom/* Shorter: %{_datadir}/atom/ The trailing slash would not be needed, but makes it more explicit that it's a directory to be included, not a single file. %dir only is helpful if you want to include the entry for a directory but specify individual files in it instead of the '*'-everything wildcard. Sort of: %files %dir %{_datadir}/somedir %{_datadir}/somedir/file1 %{_datadir}/somedir/file2 %{_datadir}/somedir/file3 %files subpkg1 %{_datadir}/somedir/file4 %{_datadir}/somedir/file5 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review