[Bug 1315193] Review Request (EPEL): cmake3 - Cross-platform make system

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1315193



--- Comment #8 from Björn "besser82" Esser <fedora@xxxxxxxxxxx> ---
(In reply to Antonio Trande from comment #6)
> Package Review
> ==============
> 
> Issues:
> =======
> - Have you considered this comment?
>   https://bugzilla.redhat.com/show_bug.cgi?id=1290199#c7

I took that into consideration, but I decided to keep this as cmake3, because
of some further thoughts:

It may be that minor version updates introduce backwards-incompatible changes,
but those usually can be fixed using simple patches or explicitly setting
CMake-policies in the CMakeLists.txt; most "so called backward-incompatible
changes were just merely a br0k3n-by-design CMake-script.  Most of incompatible
changes (except for the change from 2.8 to 3.0 with setting PREFIX in
CMakeLists.txt) can be explicitly reverted or ernabled by using the proper
CMake-policies.  I'm using CMake quite a lot and since years (since CMake 2.6)
for building my code inside different environments and there was *never* a
change, that br0k3 anything in a way, which wasn't fixable within less that 5
minutes.


> - Fix this line, please:
>   BuildRequires:  /usr/bin/sphinx-build

Why?  Any guideline out the telling me, I must not require a certain thing by
it's file-name?  Same would apply for the Fedora-package, which is exactly like
this.


> - License files are not marked with %license

Is that a requirement for EPEL by guidelines?  %license hasn't been present
when epel6 or epel7 were released…  On epel %license does nothing, but the same
archived using %doc…


> - -doc sub-package is NOT noarched, also contains
>   license files only. Missing documentation files.
>   It shouldn't requires main package:
> 
> Requires: %{name} = %{version}-%{release}
> 
>   If it just provides documentation files, should be a
>   stand-alone package.

This is taken excactly 1 by 1 from the Fedora master-branch of CMake…  I don't
want to have a too noisy diff between them two…  I'm going to discuss the
change with orion, so we can get that sorted out later…


> - appdata is not validated.
>   https://fedoraproject.org/wiki/Packaging:AppData#app-data-validate_usage
> 
>   Also it points to a bad .desktop file. I suggest to rename these files as
>   cmake3 (package name):
> 
>   cmake3.desktop
>   cmake3.appdata.xml
>   -->   <id type="desktop">cmake3.desktop</id>
>   --> <application> tags are obsoleted
>   See
> https://fedoraproject.org/wiki/Packaging:AppData#.appdata.xml_file_creation

AppData isn't even created nor used on current epel; it is conditionalized.


> - This package contains emacs related files. I guess we can apply emacs
> packaging guidelines (Case 2).
>  
> http://fedoraproject.org/wiki/Packaging:
> Emacs#Important_notes_on_these_Guidelines

Okay will fix that…


> - %{_libdir}/%{name} is empty.

It's intentionally empty.  This directory will hold the exported targets
generated by CMake-projects, which can be imported into other CMake-projects
using `find_package(Foo)`

> - BuildRoot is just for EPEL5.

Will squash that out…

> - Use %global
> 
>   %define qt_gui --qt-gui

Same here.

> - Some minor warnings in rpmlint's outputs
>   (non-executable-script, incoherent-version-in-changelog).
> 
> ===== Additional issues ======
> 
> - All binary files are Partial Relro and with PIE disabled.

epel7 nor epel6 ever used PIE/PIC nor read-only relocations.  That change came
in Fedora 23, which was after the release of epel7.  If epel7 would require any
of those flags, they would be exported by default from %{optflags} ||
%{__global_ldflags}.


> [!]: License file installed when any subpackage combination is installed.

Huh?  All licenses are in the main-pkg…  If I install any of the other
packages, the main-pkg is pulled in…


> [!]: If the package is under multiple licenses, the licensing breakdown
>      must be documented in the spec.

Documentation is in spec-file:
  # most sources are BSD
  # Source/CursesDialog/form/ a bunch is MIT
  # Source/kwsys/MD5.c is zlib
  # some GPL-licensed bison-generated files, these all include an
    exception granting redistribution under$

The other files are simply inside the archive for bootstrapping purposes and
are not build nor included in any resulting binary-package…


> [-]: Development files must be in a -devel package
> [?]: Package uses nothing in %doc for runtime.

No runtime-files in doc…


> [?]: Package is named according to the Package Naming Guidelines.
> [?]: Package does not generate any conflict.

Try to install it along "regular cmake"…  No clashes.


> [-]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [!]: Requires correct, justified where necessary.

Huh?  What's wrong with the Requires of the resulting binary packages?


> [?]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 30720 bytes in 9 files.

?


> [?]: Buildroot is not present
>      Note: Buildroot: present but not needed

Will fix that…


> [!]: Final provides and requires are sane (see attachments).
> [!]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
>      cmake3-doc , cmake3-gui , cmake3-debuginfo

Will fix that…


> Generic:
> [ ]: Large data in /usr/share should live in a noarch subpackage if package
>      is arched.

That change will go hand-in-hand with the possible change in Fedora's
cmake.spec.

-- 
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




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]