Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Few spec file fixes #191

Closed
wants to merge 15 commits into from

Conversation

Projects
None yet
2 participants
@lslebodn
Copy link
Contributor

commented Mar 10, 2017

@lslebodn lslebodn force-pushed the lslebodn:spec_fixes branch 2 times, most recently from fcc8668 to f51bc12 Mar 10, 2017

@lslebodn

This comment has been minimized.

Copy link
Contributor Author

commented Mar 10, 2017

@lslebodn lslebodn force-pushed the lslebodn:spec_fixes branch from f51bc12 to 2cb1c4b Apr 7, 2017

@lslebodn

This comment has been minimized.

Copy link
Contributor Author

commented Apr 7, 2017

Patches were updated due to sssd-kcm and libsss_certmap.

@lslebodn

This comment has been minimized.

Copy link
Contributor Author

commented Apr 7, 2017

@fidencio fidencio self-assigned this May 2, 2017

@fidencio

This comment has been minimized.

Copy link
Contributor

commented May 2, 2017

  • SPEC: Update processing of translation in %install - ACK
  • SPEC: Move systemd service sssd-ifp.service to right package - Please, remove the "Explanation" word from the commit message
  • SPEC: Add missing scriptlets for package sssd-dbus - ACK
  • SPEC: Use correct package for translated sssd-ifp man page - ACK
  • SPEC: Move man page for sss_rpcidmapd to the right package - ACK
  • SPEC: Use correct package for translated sss_ssh* man pages - ACK
  • SPEC: Use correct package for translated sssctl man pages - ACK
  • SPEC: Use correct package for translated idmap_sss man pages - ACK
  • SPEC: Use correct package for translated sss_certmap man pages - ACK
  • SPEC: Use correct package for translated sssd-kcm man pages - ACK
  • SPEC: Move files provider files within package - Please, first line of the commit message fits in the 74 chars, no need to put it in a different line
  • SPEC: Move kcm scriptlets to systemd section - ACK
  • SPEC: Call ldconfig in libsss_certmap scriptlets - ACK
  • SPEC: Use macro python_provide conditionally - There are a few typos in the commit message:
    • This is a reason -> This is the reason
    • defind -> defined
    • This is a reason why it need to be use conditionaly (...) -> This is the reason why it has to be used conditionally (...)
  • SPEC: Use %license macro - In which Fedora/CentOS version we have it introduced? I'm just asking to be sure it won't cause any issue with people building our git master with ancient (but still supported) systems.
@fidencio

This comment has been minimized.

Copy link
Contributor

commented May 2, 2017

Setting the label "Changes requested" accordingly to the review.

Thanks for the patch set, quite nice one!

Lukas Slebodnik added some commits Mar 10, 2017

Lukas Slebodnik
SPEC: Move systemd service sssd-ifp.service to right package
The sssd-ifp.service was installed even though sssd_ifp
was not installed on systemd.
Lukas Slebodnik
SPEC: Move man page for sss_rpcidmapd to the right package
Patch also fixes location of translated manual pages

Resolves:
https://pagure.io/SSSD/sssd/issue/3327
Lukas Slebodnik
SPEC: Use correct package for translated sss_certmap man pages
This patch also moved sss_certmap.5 from sssd-common to libsss_certmap

Resolves:
https://pagure.io/SSSD/sssd/issue/3327
Lukas Slebodnik
SPEC: Move files provider files within package
It's a cosmetic change to group similar files together (e.g. man pages).

The same order is in fedora downstream spec file.
It simplify comparison of changes between spec files.
Lukas Slebodnik
SPEC: Call ldconfig in libsss_certmap scriptlets
We do it for other libraries.
Lukas Slebodnik
SPEC: Use macro python_provide conditionally
The rpm macro python_provide is defined only in fedora and epel.
This is the reason why we have fallback definition in the beginning of
spec file otherwise build on rhel would fail.

This macro is defined in file /usr/lib/rpm/macros.d/macros.python
provided by package python-rpm-macros.

  sh$ rpm -qf /usr/lib/rpm/macros.d/macros.python
  python-rpm-macros-3-20.fc26.noarch

  sh$ grep python_provide /usr/lib/rpm/macros.d/macros.python
  %python_provide() %{lua:
      print("%python_provide: ERROR: ")

But this package is not installed in minimal chroot and therefore
build dependencies cannot be extracted from spec file.

  sh$ mock --clean --shell 'rpm -q python-rpm-macros' 2>/dev/null
  package python-rpm-macros is not installed

  sh$ mock --shell 'rpm --eval "%{python_provide python-test}"' 2>/dev/null
  %{python_provide python-test}

  sh$ mock --resultdir . --rebuild sssd-1.15.3-0.fc26.src.rpm
  ...
  error: line 295: Unknown tag: %{python_provide python2-sssdconfig}
  ...

This is the reason why it has to be used conditionally in fedora as it is shown
in example common spec file in python fedora packaging guidelines
  http://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file

  sh$ rpm -q --whatrequires python-rpm-macros
  python2-devel-2.7.13-5.fc26.x86_64
  python3-devel-3.6.0-22.fc26.x86_64

This patch reduce differences between upstream and fedora spec file.
Lukas Slebodnik
SPEC: Use %license macro
Starting with rpm 4.11, it is possible to install the license using
a new file macro %license, this will separate the license files from documents
and install them in a special directory in /usr/share

  rpm -q -l -p ./sssd-1.15.3-0.el7.x86_64.rpm
  /usr/share/licenses/sssd-1.15.3
  /usr/share/licenses/sssd-1.15.3/COPYING

@lslebodn lslebodn force-pushed the lslebodn:spec_fixes branch from fe99f63 to 7e0cce4 May 2, 2017

@lslebodn

This comment has been minimized.

Copy link
Contributor Author

commented May 2, 2017

@fidencio

This comment has been minimized.

Copy link
Contributor

commented May 2, 2017

  • SPEC: Move files provider files within package - There's a typo in the commit message that I didn't notice before (sorry). it simplify -> It simplifies ...
  • SPEC: Use %license macro - In the patch ... "must be define alter ..." -> "must be defined after ..."

Please, just change those typos before pushing, there's no need to resend the series because of this change.

@fidencio fidencio added Accepted and removed Changes requested labels May 2, 2017

@lslebodn

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2017

SPEC: Use %license macro - In the patch ... "must be define alter ..." -> "must be defined after ..."

I found another typo in this patch occurence vs occurrence

master:

@lslebodn lslebodn added the Pushed label May 3, 2017

@lslebodn lslebodn closed this May 3, 2017

@lslebodn lslebodn deleted the lslebodn:spec_fixes branch May 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.