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

Packaging for Inclusion in Fedora Linux #58

Closed
3 of 7 tasks
cottsay opened this issue Aug 30, 2017 · 5 comments
Closed
3 of 7 tasks

Packaging for Inclusion in Fedora Linux #58

cottsay opened this issue Aug 30, 2017 · 5 comments
Assignees

Comments

@cottsay
Copy link

cottsay commented Aug 30, 2017

Hi!

This package is a candidate for inclusion as an official package in Fedora Linux! Per Fedora packaging guidelines, there are some issues that should be addressed before the package can be officially reviewed, built, and made available in the Fedora RPM repositories.

Since this package is part of a group of packages, I have been prototyping the packages in a Fedora Copr repository [1]. The preliminary sources for this package can be accessed in a Git repository [2].

Here are the initial issues which should be addressed:

  • Upstream package(s)
    Since this package needs libpotassco and clasp, we should prioritize addressing issues with those packages first [3] [4].
  • .SO Name Versioning
    Fedora guidelines strongly suggest that the version be included in any shared object libraries distributed in the package [5]. Some packages in this collection appear to be setting the SONAME value, though the value doesn't match the version of the package.
  • Installation Directory Override
    Any files installed by the packages must conform to Fedora's filesystem hierarchy. In order for this package to install the files as such, it must not override CMake's default install locations without any mechanism to change them. I have sketched out a preliminary patch to address this [6]. Please feel free to use those changes as you see fit.
  • Shared Library By Default
    When CMake is invoked during Fedora packaging, CMake is configured to build shared libraries instead of static ones by default. It appears that this breaks a couple of the libraries in this package, which I believe are intended to always be static, and not distributed in the package. Assuming this is the case, I made a patch to specifically build those libraries as static [7].
  • Unbundle from clasp
    From what I can tell, efforts were made such that clasp can be built separately from libpotassco. Those same patterns weren't used in this package, but I was able to apply the same technique to achieve this [8].
  • Build API documentation
    It would be great to package the API documentation for offline consumption like clasp does. It looks like this is an easy thing to enable [9].
  • Python and Lua Dependencies
    I haven't had a chance to fully investigate this, but it appears that when I build clingo with Lua and Python modules enabled, the core application is linked against those libraries. It should be possible to build those modules, but install them optionally. Right now, installing clingo without the modules still pulls Lua and Python as dependencies. I suspect this isn't intentional, and could be addressed by a change to the CMakeLists.txt.

Once we have at least acknowledged each of the above issues, I'll initiate the review. If there is a valid reason that any of the issues can't be addressed, please let me know and we'll talk about an FPC exemption. If you have any further questions, or if there is anything I can do to help, please reach out.

Thanks very much!

--scott

[1] https://copr.fedorainfracloud.org/coprs/cottsay/potassco
[2] http://copr-dist-git.fedorainfracloud.org/cgit/cottsay/potassco/clingo.git/tree/
[3] potassco/libpotassco#1
[4] potassco/clasp#12
[5] https://fedoraproject.org/wiki/Packaging:Guidelines#Downstream_.so_name_versioning
[6] http://copr-dist-git.fedorainfracloud.org/cgit/cottsay/potassco/clingo.git/tree/clingo-5.2.1-install_dir.patch
[7] http://copr-dist-git.fedorainfracloud.org/cgit/cottsay/potassco/clingo.git/tree/clingo-5.2.1-default_shared.patch
[8] http://copr-dist-git.fedorainfracloud.org/cgit/cottsay/potassco/clingo.git/tree/clingo-5.2.1-unbundle_clasp.patch
[9] http://copr-dist-git.fedorainfracloud.org/cgit/cottsay/potassco/clingo.git/tree/clingo-5.2.1-doc_clingo.patch

@rkaminsk
Copy link
Member

rkaminsk commented Sep 6, 2017

Hi, I am currently on vacation but I will come back to this next week. Also have a look at the following: https://github.com/rkaminsk/clingo-srpm

@rkaminsk
Copy link
Member

rkaminsk commented Sep 7, 2017

Hi @cottsay,

first of all thanks for putting so much effort in packaging our tools!

Unbundling clasp and libpotassco is the biggest issue here. Both projects have never been developed with use as shared libraries in mind. Probably, they will work if build/linked dynamically. The resulting shared objects will be messy though. It would be a major issue to address this, which will not happen anytime soon.

One very easy way out would be to derive the clasp package from the clingo project too. Would this be okay? Note that clasp is not really a third party project but developed by the same group. @BenKaufmann what do you think about this?

.SO Name Versioning

The idea here was to increment the version number whenever the API is no longer backward compatible. Since clingo is not just a library the version number might increment without breaking backward compatibily. Hence, the idea was to use separate version numbers.

Installation Directory Override

The wip branch already contains a variable CLINGO_LIBRARY_DESTINATION. Do you also need the same for include as in your patch?

Shared Library By Default

I am going to apply the changes you propose here. These libraries are not intended to be linked dynamically.

Build API documentation

I thought about adding this to the cmake files. But decided that it is too ugly. I exclusively do out of source builds with cmake. Your patch does not do this and doxygen makes it hard to implement this. Why not simply add cd doc/api; doxygen to the spec file?

Python and Lua Dependencies

Please have a look at my previous comment where modules for lua, python2 and python3 are build. Also note that clingo itself is linked against lua and python3 there. AFAIK Fedora's policy is to push python3 and I see no reason not to enable python3 by default in clingo.

Could you simply incorporate this in you specfile? Here one has to be careful with building libpyclingo dynamically. The python3 version could be build dynamically. The python2 version must not be build dynamically, which is okay because it is only used by the python2 module anyway.

@cottsay
Copy link
Author

cottsay commented Sep 7, 2017

Thanks so much for looking into this, @rkaminsk! I'm actually going on vacation until next week as well. I'll take a look at this as soon as I return.

rkaminsk added a commit that referenced this issue Sep 8, 2017
@rkaminsk rkaminsk self-assigned this Oct 7, 2017
@rkaminsk
Copy link
Member

Since there was no activity on this for more than half a year, I am going to close this. If there are plans to continue on this just reopen.

@tkren
Copy link

tkren commented Apr 13, 2018

@cottsay: as a follow-up, clasp and gringo is part of Debian/Ubuntu, there might be some common code that can be shared:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants