Bug 1107441

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (3 clause)", "Unknown or generated", "zlib/libpng". 7 files have
     unknown license. Detailed output of licensecheck in
     /home/flo/review/1107441-udt/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: %build honors applicable compiler flags or justifies otherwise.
   ---> Did you report upstream about not supporting _smp_mflags?
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
   ---> Please remove 'rm -rf %{buildroot}'
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: 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]: Useful -debuginfo package or justification otherwise.
[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 235520 bytes in 63 files.
[!]: Package complies to the Packaging Guidelines
   ---> some minor issues 
[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: No rpmlint messages.
[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 %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[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]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[!]: Permissions on files are set properly.
   ---> Please replase 'install -p -m 644' with 'install -pm 0644'
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[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

===== SHOULD items =====

Generic:
[!]: Uses parallel make %{?_smp_mflags} macro.
[-]: 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]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in udt-devel
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: 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.
    ---> http://koji.fedoraproject.org/koji/taskinfo?taskID=7040799
[-]: %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]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[!]: SourceX is a working URL.
   ---> please append '#/%{name}-%{version}.tar.gz' to Source0, to get
        a properly named source-tarball
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: udt-4.11-1.fc21.x86_64.rpm
          udt-devel-4.11-1.fc21.x86_64.rpm
          udt-4.11-1.fc21.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint udt-devel udt
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
udt-devel (rpmlib, GLIBC filtered):
    libudt.so.0()(64bit)
    udt

udt (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.1)(64bit)
    rtld(GNU_HASH)



Provides
--------
udt-devel:
    udt-devel
    udt-devel(x86-64)

udt:
    libudt.so.0()(64bit)
    udt
    udt(x86-64)



Source checksums
----------------
http://downloads.sourceforge.net/project/udt/udt/4.11/udt.sdk.4.11.tar.gz :
  CHECKSUM(SHA256) this package     : aa25b6d7cbac474ca05b7c7b36f59e9a3cd5c61faed8bf1b7174ac118c3de1db
  CHECKSUM(SHA256) upstream package : aa25b6d7cbac474ca05b7c7b36f59e9a3cd5c61faed8bf1b7174ac118c3de1db


Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1107441
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

===== Solution =====
NOT approved.  Please fix-up those named issues and I'll have another review.

(In reply to Florian "der-flo" Lehner from comment #1)
> [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
>      Note: rm -rf %{buildroot} present but not required
>    ---> Please remove 'rm -rf %{buildroot}'

Sorry, I should have mentioned that I am planning to build this for EPEL 5.
(I know I didn't do this properly - I need to add some additional stuff that is no longer part of the emacs generated template to get it to properly build on EPEL 5.)

> [!]: Permissions on files are set properly.
>    ---> Please replace 'install -p -m 644' with 'install -pm 0644'

Why is "-pm" better than "-p -m"? Personally I think that stacking all option together instead of listing them separately obscures what you are doing and makes the call harder to understand. This is especially true in case one of the options takes an argument as in this case.

> [!]: SourceX is a working URL.
>    ---> please append '#/%{name}-%{version}.tar.gz' to Source0, to get
>         a properly named source-tarball

Is this a serious complaint? The name of the upstream tarball is important for identifying the upstream sources. Arbitrarily renaming it looses the pedigree of the file, and gives rise to questions about why is the tarball named differently from upstream's name in the source RPM? Has it been modified in some way? I disagree with this suggestion.


New version that fixes the EPEL 5 build:

Spec URL: http://www.grid.tsl.uu.se/review/udt.spec
SRPM URL: http://www.grid.tsl.uu.se/review/udt-4.11-2.fc20.src.rpm

(In reply to Mattias Ellert from comment #2)
> > [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
> >      beginning of %install.
> >      Note: rm -rf %{buildroot} present but not required
> >    ---> Please remove 'rm -rf %{buildroot}'
> 
> Sorry, I should have mentioned that I am planning to build this for EPEL 5.
> (I know I didn't do this properly - I need to add some additional stuff that
> is no longer part of the emacs generated template to get it to properly
> build on EPEL 5.)

Please clarify this by a comment above the line

> 
> > [!]: Permissions on files are set properly.
> >    ---> Please replace 'install -p -m 644' with 'install -pm 0644'
> 
> Why is "-pm" better than "-p -m"? Personally I think that stacking all
> option together instead of listing them separately obscures what you are
> doing and makes the call harder to understand. This is especially true in
> case one of the options takes an argument as in this case.

I see your point and I agree with you.
It's the leading 0 of 0644 that I wanted to point to. Using 0644 instead of 644 is just a good style.

> > [!]: SourceX is a working URL.
> >    ---> please append '#/%{name}-%{version}.tar.gz' to Source0, to get
> >         a properly named source-tarball
> 
> Is this a serious complaint? The name of the upstream tarball is important
> for identifying the upstream sources. Arbitrarily renaming it looses the
> pedigree of the file, and gives rise to questions about why is the tarball
> named differently from upstream's name in the source RPM? Has it been
> modified in some way? I disagree with this suggestion.

Appending '#/%{name}-%{version}.tar.gz' to Source0, will let you get a properly named source-tarball After this change the use of `spectool -g [-R] udt.spec` will give you a properly named src-tarball automatically.
---> https://fedoraproject.org/wiki/Packaging:SourceURL#Troublesome_URLs

(In reply to Florian "der-flo" Lehner from comment #3)
> (In reply to Mattias Ellert from comment #2)
> > > [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
> > >      beginning of %install.
> > >      Note: rm -rf %{buildroot} present but not required
> > >    ---> Please remove 'rm -rf %{buildroot}'
> > 
> > Sorry, I should have mentioned that I am planning to build this for EPEL 5.
> > (I know I didn't do this properly - I need to add some additional stuff that
> > is no longer part of the emacs generated template to get it to properly
> > build on EPEL 5.)
> 
> Please clarify this by a comment above the line

I have added a comment to the specfile.

> > 
> > > [!]: Permissions on files are set properly.
> > >    ---> Please replace 'install -p -m 644' with 'install -pm 0644'
> > 
> > Why is "-pm" better than "-p -m"? Personally I think that stacking all
> > option together instead of listing them separately obscures what you are
> > doing and makes the call harder to understand. This is especially true in
> > case one of the options takes an argument as in this case.
> 
> I see your point and I agree with you.
> It's the leading 0 of 0644 that I wanted to point to. Using 0644 instead of
> 644 is just a good style.

I don't believe there is consensus about this. I for one have the opposite opinion and find the version with the 0 in front slightly more confusing. Or at least used to before I knew the meaning of the forth digit. I usually avoid using the fourth digit unless really necessary (and it rarely is since you rarely need to set the setuid/setgid bits on files).

I agree that this is a matter of style, but I do not believe it is a matter of bad vs. good style and that one style must be used and the other must not.

> > > [!]: SourceX is a working URL.
> > >    ---> please append '#/%{name}-%{version}.tar.gz' to Source0, to get
> > >         a properly named source-tarball
> > 
> > Is this a serious complaint? The name of the upstream tarball is important
> > for identifying the upstream sources. Arbitrarily renaming it looses the
> > pedigree of the file, and gives rise to questions about why is the tarball
> > named differently from upstream's name in the source RPM? Has it been
> > modified in some way? I disagree with this suggestion.
> 
> Appending '#/%{name}-%{version}.tar.gz' to Source0, will let you get a
> properly named source-tarball After this change the use of `spectool -g [-R]
> udt.spec` will give you a properly named src-tarball automatically.
> ---> https://fedoraproject.org/wiki/Packaging:SourceURL#Troublesome_URLs

The guideline you are referring to is an exception that only should be applied when specific condition are met, The condition stated in the guideline for when it should be applied is not met in this case.

The condition for when to apply this guideline as stated in the guideline itself is "When upstream has URLs for the download that do not end with the tarball name". In this case the URL does end with the name of the tarball. So this exception is not applicable.

Applying the exceptional guideline when the condition for applying it is not met would be a guideline violation. I would consider it to be a severe guideline violation and if I would review a package that did that I would block it until it was removed. Since you are asking me to do something that would make me block my own package review I will not do that.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (3 clause)", "Unknown or generated", "zlib/libpng". 7 files have
     unknown license. Detailed output of licensecheck in
     /home/flo/review/1107441-udt/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
   ---> clarified by a comment
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: 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]: Useful -debuginfo package or justification otherwise.
[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 235520 bytes in 63 files.
[x]: Package complies to the Packaging Guidelines
   ---> http://koji.fedoraproject.org/koji/taskinfo?taskID=7051522
[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: No rpmlint messages.
[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 %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[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]: Macros in Summary, %description expandable at SRPM build time.
[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 do not use a name that already exist
[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

===== SHOULD items =====

Generic:
[x]: Uses parallel make %{?_smp_mflags} macro.
   ---> clarified by a comment
[x]: Buildroot is not present
     Note: Buildroot: present but not needed
   ---> clarified by a comment
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required
[-]: 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]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in udt-devel
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: 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.
   ---> http://koji.fedoraproject.org/koji/taskinfo?taskID=7051522
[-]: %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]: Dist tag is present (not strictly required in GL).
[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.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: udt-4.11-2.fc21.x86_64.rpm
          udt-devel-4.11-2.fc21.x86_64.rpm
          udt-4.11-2.fc21.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint udt-devel udt
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
udt-devel (rpmlib, GLIBC filtered):
    libudt.so.0()(64bit)
    udt

udt (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.1)(64bit)
    rtld(GNU_HASH)



Provides
--------
udt-devel:
    udt-devel
    udt-devel(x86-64)

udt:
    libudt.so.0()(64bit)
    udt
    udt(x86-64)



Source checksums
----------------
http://downloads.sourceforge.net/project/udt/udt/4.11/udt.sdk.4.11.tar.gz :
  CHECKSUM(SHA256) this package     : aa25b6d7cbac474ca05b7c7b36f59e9a3cd5c61faed8bf1b7174ac118c3de1db
  CHECKSUM(SHA256) upstream package : aa25b6d7cbac474ca05b7c7b36f59e9a3cd5c61faed8bf1b7174ac118c3de1db


Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1107441
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

===== Solution =====
APPROVED

Many thanks for the review.

New Package SCM Request
=======================
Package Name: udt
Short Description: UDP based Data Transfer Protocol
Owners: ellert
Branches: f19 f20 master el5 el6 el7
InitialCC:

Good work, Flo! There are just two small things I want to mention, additionally:

[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in udt-devel

     ---> please fix up the requires of the -devel-subpkg.


[!]: Packages should try to preserve timestamps of original installed files.

     ---> `sed 's/\r//' -i doc/doc/udtdoc.css` doesn't preseve the timestamp
          of that particular file with will be packaged in -devel.  ;)

          Using something like this would be better by the meaning of preserving
          the file's timestamp:

          _file="doc/doc/udtdoc.css"
          sed -e 's!\r$!!g' < ${_file} > ${_file}.new && \
          touch -r ${_file} ${_file}.new && \
          mv -f ${_file}.new ${_file}


Please change those two small accordingly before / during import, Matthias.

btw. The branch used for building EPEL-pkg for rhel / CentOS 7 is called 'epel7'.  ;)  I just fixed your scm-request, Matthias. ^^

New Package SCM Request
=======================
Package Name: udt
Short Description: UDP based Data Transfer Protocol
Owners: ellert
Branches: f19 f20 master el5 el6 epel7
InitialCC:

Git done (by process-git-requests).

(In reply to Björn "besser82" Esser from comment #7)
> Good work, Flo! There are just two small things I want to mention,
> additionally:
> 
> [!]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in udt-devel
> 
>      ---> please fix up the requires of the -devel-subpkg.

This is already there. I suspect you were confused by the requires as reported by fedora-review. This tool fails to report versions in the requires. It does the check for it properly, and complains if it is not there (and there were no complaint in this case). But the list it displays is a bit confusing because it does not contain the versions. Someone should probably file a bug report to fedora-review about this.

> [!]: Packages should try to preserve timestamps of original installed files.
> 
>      ---> `sed 's/\r//' -i doc/doc/udtdoc.css` doesn't preseve the timestamp
>           of that particular file with will be packaged in -devel.  ;)
> 
>           Using something like this would be better by the meaning of
> preserving
>           the file's timestamp:
> 
>           _file="doc/doc/udtdoc.css"
>           sed -e 's!\r$!!g' < ${_file} > ${_file}.new && \
>           touch -r ${_file} ${_file}.new && \
>           mv -f ${_file}.new ${_file}

The file that is installed is not the original file since it is modified, so giving the modified file the timestamp of the original seems a bit strange. That would give the impression that it is the original file that is installed. If I would do a similar fix using a %patch I would not be able to do that anyway, and it seems strange to do different things with the timestamps depending on how the file was modified.

http://fedoraproject.org/wiki/Packaging:Guidelines#Rpmlint_Errors

says "Fix it in the %prep section with sed: sed -i 's/\r//' src/somefile"

http://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-encoding

gives the same advise.

(In reply to Björn "besser82" Esser from comment #8)
> btw. The branch used for building EPEL-pkg for rhel / CentOS 7 is called
> 'epel7'.  ;)  I just fixed your scm-request, Matthias. ^^

Thank you.
PS. You keep adding an h in my name)

udt-4.11-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/udt-4.11-2.fc20

udt-4.11-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/udt-4.11-2.el6

udt-4.11-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/udt-4.11-2.el5

udt-4.11-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/udt-4.11-2.fc19

(In reply to Mattias Ellert from comment #10)
> (In reply to Björn "besser82" Esser from comment #7)
> > Good work, Flo! There are just two small things I want to mention,
> > additionally:
> > 
> > [!]: Fully versioned dependency in subpackages if applicable.
> >      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in udt-devel
> > 
> >      ---> please fix up the requires of the -devel-subpkg.
> 
> This is already there. I suspect you were confused by the requires as
> reported by fedora-review. This tool fails to report versions in the
> requires. It does the check for it properly, and complains if it is not
> there (and there were no complaint in this case). But the list it displays
> is a bit confusing because it does not contain the versions. Someone should
> probably file a bug report to fedora-review about this.

(%{name} = %{version}-%{release}) != (%{name}%{?_isa} = %{version}-%{release})

With the current non-arched requirement the i686-mainpkg will satisfy the dependency of the x86_64-develpkg, which will result in a dangling symlink and possibly other strange problems in chain of that.  ;)  There is a good reason about arched-requires like this to be part of the guidelines.


> > [!]: Packages should try to preserve timestamps of original installed files.
> > 
> >      ---> `sed 's/\r//' -i doc/doc/udtdoc.css` doesn't preseve the timestamp
> >           of that particular file with will be packaged in -devel.  ;)
> > 
> >           Using something like this would be better by the meaning of
> > preserving
> >           the file's timestamp:
> > 
> >           _file="doc/doc/udtdoc.css"
> >           sed -e 's!\r$!!g' < ${_file} > ${_file}.new && \
> >           touch -r ${_file} ${_file}.new && \
> >           mv -f ${_file}.new ${_file}
> 
> The file that is installed is not the original file since it is modified, so
> giving the modified file the timestamp of the original seems a bit strange.
> That would give the impression that it is the original file that is
> installed. If I would do a similar fix using a %patch I would not be able to
> do that anyway, and it seems strange to do different things with the
> timestamps depending on how the file was modified.
> 
> http://fedoraproject.org/wiki/Packaging:Guidelines#Rpmlint_Errors
> 
> says "Fix it in the %prep section with sed: sed -i 's/\r//' src/somefile"
> 
> http://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-
> encoding
> 
> gives the same advise.

http://fedoraproject.org/wiki/Packaging_tricks#Remove_DOS_line_endings

gives you the same example I used, which will preserve timestamps.  In case of documentation and just fixing line-endings, I do not consider keeping the timestamp to be strange, since you do not modify the 'meaningful' content of the file; it's just about a platform-adjustment.

When applying a patch, you can even preserve timestamps.  There are multiple ways to accomplish that.  ;)


> (In reply to Björn "besser82" Esser from comment #8)
> > btw. The branch used for building EPEL-pkg for rhel / CentOS 7 is called
> > 'epel7'.  ;)  I just fixed your scm-request, Matthias. ^^
> 
> Thank you.
> PS. You keep adding an h in my name)

np ^^  You're welcome!  Sry, about that additonal 'h'.  ;(

> With the current non-arched requirement the i686-mainpkg will satisfy the
> dependency of the x86_64-develpkg, which will result in a dangling symlink
> and possibly other strange problems in chain of that.  ;)

That never happens *if* there is a proper SONAME, because then the -devel subpkg automatically requires that one [-> Requires: libudt.so.0()(64bit)].

The case you refer to can happen with other packages (e.g. non-lib packages) or no-SONAME libs - you know this one already:
https://admin.fedoraproject.org/updates/FEDORA-2014-7438/lpsolve-5.5.2.0-8.fc20

udt-4.11-2.el6 has been pushed to the Fedora EPEL 6 testing repository.

udt-4.11-2.fc20 has been pushed to the Fedora 20 stable repository.

udt-4.11-2.fc19 has been pushed to the Fedora 19 stable repository.

udt-4.11-2.el5 has been pushed to the Fedora EPEL 5 stable repository.

udt-4.11-2.el6 has been pushed to the Fedora EPEL 6 stable repository.