https://bugzilla.redhat.com/show_bug.cgi?id=851279 --- Comment #18 from Andy Grimm <agrimm@xxxxxxxxx> --- (In reply to comment #16) > === List of issues === > > As some binaries are composed of code with multiple, compatible licenses, I > believe the license tag should be "GPLv3 and (GPLv3 and ASL 2.0) and (GPLv3 > and BSD)". It might be worth double-checking with Spot, though. Fixed. > clc/modules/www/src/licenses/webui-iconic-icons.LICENSE must be included > with %doc. I think we discussed this one offline; this isn't needed until we build the UI. > /etc/eucalyptus/cloud.d/init.d/01_pg_kernel_params appears in two packages. This file is now completely excluded from the install, as it's not really an approved way to deal with setting kernel parameters. > /etc/eucalyptus/eucalyptus.conf is owned by the eucalyptus user. If that is > actually correct (yuck) then please state so in the spec file. Added a comment. > Rather than starting tgtd in the sc package's %post scriptlet, consider > adding Wants=tgtd.service to eucalyptus-cloud.service. Fixed. > Are multiple restarts of the same service in %postun really the safest/most > reasonable way to do upgrades? I've removed all duplication of these for now. I think we need to more closely investigate the right way to upgrade this package, but you're right that restarting multiple times is awful. > axis2-clients should probably require eucalyptus-cc with a fully-versioned > dependency. Fixed. > The following files have filesystem layout problems: > These go in /usr/share/%name: > /etc/eucalyptus/cloud.d/init.d/01_pg_kernel_params > /etc/eucalyptus/eucalyptus-version > /etc/eucalyptus/vtunall.conf.template Removed the first; fixed the other two. > These go in /usr/share/%name or /usr/libexec/%name: > /etc/eucalyptus/cloud.d/scripts/* > /etc/eucalyptus/cloud.d/upgrade/* > /usr/sbin/eucalyptus-*.init Moved these into libexec, but had the symlink the cloud.d ones for now. > This goes with documentation: > /etc/eucalyptus/drbd.conf.example Moved. > The following configuration files are not marked with %config(noreplace) or > %config: > /etc/eucalyptus/axis2.xml > /etc/eucalyptus/eucalyptus.conf > /etc/eucalyptus/httpd/conf/httpd-cc.conf > /etc/eucalyptus/httpd/conf/httpd-common.conf > /etc/eucalyptus/httpd/conf/httpd-nc.conf Fixed. > %install uses %{S:2}, whereas the rest of the file uses the %{SOURCE#} > format. Fixed (I found at least one other like this as well) > /var/run/eucalyptus needs a tmpfiles.d entry. Added. > eucalyptus-cc cleanstart, cleanstop, and cleanrestart are supposed to be > converted to standalone scripts. /usr/sbin/eucalyptus-clean-cc is now a separate script which performs the "clean" function which was formerly part of the init script. > The spec file must contain BuildRequires: systemd-units for the %_unitdir > macro. Fixed. > The Java guidelines require arch-independent JARs to go under %_javadir, not > /usr/share/eucalyptus. This affects the following files: > /usr/share/eucalyptus/eucalyptus-auth-3.1.0.jar > ... Fixed. /usr/share now just has symlinks. > Similarly, JNI-using JARs must go in %_libdir/%name. There is one of these: > /usr/share/eucalyptus/eucalyptus-storagecontroller-3.1.0.jar Fixed > Calls to System.loadLibrary must be replaced with System.load with complete > .so paths. This affects the following file in the source tree: > > clc/modules/storage-controller/src/main/java/com/eucalyptus/storage/ > OverlayManager.java Fixed > /usr/lib/python2.7/site-packages/eucadmin/local.py from the python-eucadmin > package requires m2crypto and python-paramiko. However, that module is > unused, so you could also patch it out if you would like. Removed. > > === Other commentary === > > I must admit that I am not a fan of macros like %eucastatedir, which don't > really do much to enhance readability. I would rather see the real paths > inline. I have remove most of these, except for: %global eucalibexecdir %{_libexecdir}/%{name} %global eucadatadir %{_datadir}/%{name} %global eucajavalibdir %{_datadir}/%{name} %global helperdir %{_datadir}/%{name} These are here because they are things that the current upstream packages put in strange / unacceptable places, and files of distinctly different types are placed in the same directory. Keeping these notations will make it easier to move these into cleaner directory structure at some point. It's easy enough to run the srpm through rpmspec to get readable names. > Are all of those httpd modules really necessary? Nope. The list is much smaller now. Figuring out the smallest usable subset was nontrivial! > If possible, please see if you can filter out Provides and Requires for > internal libs. Done. > Please query upstream about including license files for the BSD sub-packages. Filed an issue for this upstream. I think it might be assigned to you. ;) > I'd change "Eucalyptus Enterprise Edition" to "Eucalyptus Enterprise > plugins" since there's only one version of eucalyptus now. where? I don't see reference to this in the spec. I only see it in a script in the tools directory. > provide_abi isn't necessary for a Fedora package, so I'd just remove it. Removed. > You should probably add commentary about why you're explicitly setting LANG > when you call make. This was done due to a single invalid character, and has now been removed. > Packaging guidelines: > NO - Pre-built binaries, libs removed in %prep > tools/floppy contains grub This is now truncated in prep. Left as a placeholder and to avoid having to patch the makefile. > NO - Requires correct, justified where necessary > cloud, sc should require systemd-units since they restart services This has been changed. > NO - Build honors applicable compiler flags or justifies otherwise > LDFLAGS not supplied to build TODO. > NO - PIE used for long-running/root daemons, setuid/filecap programs > Long-running daemons and setuid binaries require _hardened_build TODO. I'll post a new SPEC and SRPM later today when I finish these last two things. I just wanted to give an update here document my progress. -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review