Spec URL: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer-1.0.0-0.1.gitc0957a7.fc22.src.rpm SRPM URL: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec Description: The packaged Eclipse plugin provides UI entries to enable an alternative import mechanism, relying on discovery of projects (rather than user choice). More details at https://wiki.eclipse.org/E4/UI/Smart_Import . Fedora Account System Username: sopotc
hi, first of all welcome Suggestion for spec file you can use also %pom_add_plugin org.eclipse.tycho:tycho-maven-plugin:'${tycho-version}' . "<extensions>true</extensions>" regards
I'm not familiar enough with eclipse to take this review but with a drive by spec review it looks pretty good. The only thing I see missing in the files section is a license file. Is one included in the source? If so the license macro should be used for f21+ (and I think epel7). If you plan to support older releases then you'll have to use a conditional. Something like: %if 0%{?rhel} || 0%{?fedora} < 21 %doc COPYING %else %license COPYING %endif And if epel7 does support the license macro then a "< 7" can be added to the rhel portion.
(In reply to gil cattaneo from comment #1) Seem you have duplicate tycho-maven-plugin entries in the main pom file ... is not a good idea ...
I'll take this one. Sopot would you please apply gil's suggestion. Regarding license - eclipse.org projects usually have license injected at build time in the builds via special feature developed for the purpose (as EPL is mandated ). So this is not an issue. Once gil's suggestion is applied I'll do full review.
Thank you for the prompt suggestoins. (In reply to gil cattaneo from comment #3) > (In reply to gil cattaneo from comment #1) > Seem you have duplicate tycho-maven-plugin entries in the main pom file ... > is not a good idea ... Fixed: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec
(In reply to Sopot Cela from comment #5) > Thank you for the prompt suggestoins. > > (In reply to gil cattaneo from comment #3) > > (In reply to gil cattaneo from comment #1) > > Seem you have duplicate tycho-maven-plugin entries in the main pom file ... > > is not a good idea ... > > Fixed: > https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec Sorry but you maybe use '${tycho-version}' (with single quotation mark) otherwise you must use ${tycho-version} as global variable, because the version of this plugin maybe remain empty
(In reply to gil cattaneo from comment #6) > (In reply to Sopot Cela from comment #5) > > Thank you for the prompt suggestoins. > > > > (In reply to gil cattaneo from comment #3) > > > (In reply to gil cattaneo from comment #1) > > > Seem you have duplicate tycho-maven-plugin entries in the main pom file ... > > > is not a good idea ... > > > > Fixed: > > https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec > Sorry but you maybe use '${tycho-version}' (with single quotation mark) > otherwise you must use ${tycho-version} as global variable, because the > version of this plugin maybe remain empty I tried initially as you suggested with single quotation marks but I got: [INFO] Scanning for projects... [ERROR] The build could not read 1 project -> [Help 1] [ERROR] [ERROR] The project org.eclipse.e4.ui:e4-ui-aggregator:0.17.0-SNAPSHOT (/home/rtest/rpmbuild/BUILD/org.eclipse.e4.ui-c0957a7a7d53655ecf9ae5047a94fe20de0e5d5d/pom.xml) has 1 error [ERROR] 'build.plugins.plugin.version' for org.eclipse.tycho:tycho-maven-plugin must be a valid version but is '${tycho-version}'. @ line 10, column 18 [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException Shall I remove the version at all since it is optional? It builds fine like that.
Uploaded version with no version at all.
(In reply to Sopot Cela from comment #7) > (In reply to gil cattaneo from comment #6) > > (In reply to Sopot Cela from comment #5) > > > Thank you for the prompt suggestoins. > > > > > > (In reply to gil cattaneo from comment #3) > > > > (In reply to gil cattaneo from comment #1) > > > > Seem you have duplicate tycho-maven-plugin entries in the main pom file ... > > > > is not a good idea ... > > > > > > Fixed: > > > https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec > > Sorry but you maybe use '${tycho-version}' (with single quotation mark) > > otherwise you must use ${tycho-version} as global variable, because the > > version of this plugin maybe remain empty > > I tried initially as you suggested with single quotation marks but I got: > > [INFO] Scanning for projects... > [ERROR] The build could not read 1 project -> [Help 1] > [ERROR] > [ERROR] The project org.eclipse.e4.ui:e4-ui-aggregator:0.17.0-SNAPSHOT > (/home/rtest/rpmbuild/BUILD/org.eclipse.e4.ui- > c0957a7a7d53655ecf9ae5047a94fe20de0e5d5d/pom.xml) has 1 error > [ERROR] 'build.plugins.plugin.version' for > org.eclipse.tycho:tycho-maven-plugin must be a valid version but is > '${tycho-version}'. @ line 10, column 18 > [ERROR] > [ERROR] To see the full stack trace of the errors, re-run Maven with the -e > switch. > [ERROR] Re-run Maven using the -X switch to enable full debug logging. > [ERROR] > [ERROR] For more information about the errors and possible solutions, please > read the following articles: > [ERROR] [Help 1] > http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException > > Shall I remove the version at all since it is optional? It builds fine like > that. Yes is normal if in the pom file "tycho-version" is not declared (in the properties section) Sorry, My fault i don't know how that is set pom file. I have deceived your first draft of the modification on the same file.
Last question why you set version to 1.0.0 when in the pom file is 0.17.0-SNAPSHOT ? (now, i had a look in the src archive :) ). arbitrary decision or makes sense? in the first case you must use this notation: Version: 0.17.0 Release: 0.1%{?dist} * Mon Jun 01 2015 Sopot Cela <scela> - 0.17.0-0.1
^^^ see https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages
in this case "alphatag" should be SNAPSHOT.
I updated the versions but used 0.1.0 as the version because that's the version of actual bundles (http://git.eclipse.org/c/e4/org.eclipse.e4.ui.git/tree/bundles/org.eclipse.e4.ui.importer/META-INF/MANIFEST.MF#n5 ). Also I reset the changelog. Test-build went fine. Location of SPEC and SRPM: https://sopotc.fedorapeople.org/eclipse-e4-importer/
Spec URL: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer-0.1.0-0.1.gitc0957a7.fc22.src.rpm SRPM URL: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec Full URLs to allow fedora-review to work.
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= * Rpmlint complains about description line too long. It should be less than 80. Otherwise package is good to go. ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: License field in the package spec file matches the actual license. Note: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. [x]: Package must own all directories that it creates. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local Java: [x]: Bundled jar/class files should be removed before build [x]: Packages have proper BuildRequires/Requires on jpackage-utils Note: Maven packages do not need to (Build)Require jpackage-utils. It is pulled in by maven-local Maven: [x]: If package contains pom.xml files install it (including metadata) even when building with ant [x]: POM files have correct Maven mapping [x]: Old add_to_maven_depmap macro is not being used [x]: Packages DO NOT have Requires(post) and Requires(postun) on jpackage- utils for %update_maven_depmap macro [x]: Package DOES NOT use %update_maven_depmap in %post/%postun [x]: Packages use .mfiles file list instead of %{_datadir}/maven2/poms ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. Java: [x]: Package uses upstream build method (ant/maven/etc.) [x]: Packages are noarch unless they use JNI ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: eclipse-e4-importer-0.1.0-0.1.gitc0957a7.fc23.noarch.rpm eclipse-e4-importer-0.1.0-0.1.gitc0957a7.fc23.src.rpm eclipse-e4-importer.noarch: E: description-line-too-long C This plugin provides UI entries to enable an alternative import mechanism, relying on discovery of projects (rather than user choice). eclipse-e4-importer.src: E: description-line-too-long C This plugin provides UI entries to enable an alternative import mechanism, relying on discovery of projects (rather than user choice). 2 packages and 0 specfiles checked; 2 errors, 0 warnings. Requires -------- eclipse-e4-importer (rpmlib, GLIBC filtered): eclipse-platform java-headless jpackage-utils jsch osgi(org.eclipse.jdt.core) osgi(org.eclipse.jdt.launching) osgi(org.eclipse.pde.core) osgi(org.eclipse.pde.ui) Provides -------- eclipse-e4-importer: eclipse-e4-importer mvn(org.eclipse.e4.ui:e4-ui-aggregator:pom:) mvn(org.eclipse.e4:org.eclipse.e4.ui.importer) mvn(org.eclipse.e4:org.eclipse.e4.ui.importer.java) mvn(org.eclipse.e4:org.eclipse.e4.ui.importer.pde) osgi(org.eclipse.e4.ui.importer) osgi(org.eclipse.e4.ui.importer.java) osgi(org.eclipse.e4.ui.importer.pde) Source checksums ---------------- http://git.eclipse.org/c/e4/org.eclipse.e4.ui.git/snapshot/org.eclipse.e4.ui-c0957a7a7d53655ecf9ae5047a94fe20de0e5d5d.tar.gz : CHECKSUM(SHA256) this package : 97738fbac80100411fbcaa515dbf010bbf0dd902a094fdf97bba34d483fffdc8 CHECKSUM(SHA256) upstream package : 97738fbac80100411fbcaa515dbf010bbf0dd902a094fdf97bba34d483fffdc8
Changed the description to be shorter than 80 chars. Spec URL: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer-0.1.0-0.2.gitc0957a7.fc22.src.rpm SRPM URL: https://sopotc.fedorapeople.org/eclipse-e4-importer/eclipse-e4-importer.spec
Thanks. This package is APPROVED.
Sopot, I have sponsored you now. Please open scm request.
New Package SCM Request ======================= Package Name: eclipse-e4-importer Short Description: Alternative importer of Eclipse projects Upstream URL: https://www.eclipse.org/e4/ Owners: sopotc Branches: f22 InitialCC: eclipse-sig
Git done (by process-git-requests).