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

Added packaging using CPack and pkg-config. #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KOLANICH
Copy link

Fixed installation dir of configs - it must be arch-independent for a header-only library.

@adishavit
Copy link
Owner

adishavit commented Jul 22, 2021

Thank you for your contribution!!

I have no idea what you did - I am not familiar with CPack.
Can you explain more?
Will it affect any other builds?
For other platforms - e.g. on Windows?

@KOLANICH
Copy link
Author

KOLANICH commented Jul 22, 2021

It will affect all builds. I.e. one can generate installers for Windows (using NSIS, WiX and Qt installer Framework) since now (but I don't recommend it, for such libs just a zip archive is better, of course it can also be generated).

If you use MSYS2, its pkg-config should work too, when the resulting archive is unpacked properly.

Currently CPack doesn't support pacman (a package manager used in Arch Linux distro, but it is cross-platform, so MSYS2 uses it too), which is used in MSYS2, so no package installable with it can be generated out of the box.

@adishavit
Copy link
Owner

Is this ready to merge?
BTW, is there a way to add CI for this?

@adishavit
Copy link
Owner

Pinging @a4z @BillyONeal @memsharded @sehe (et al).
You've all made PRs to argh and I would appreciate your review/inputs about this as you are more versed than I in package management.

Copy link
Contributor

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not an expert in CPack, but I'd say it is looking good. A different discussion is if the packaging specific code should belong to the library source repo or to the package system, but this would be a difficult one, and I don't think this is a discussion that belongs here.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated

if(CMAKE_SYSTEM_NAME STREQUAL Linux)
# this might be a bit too restrictive, since for other (BSD, ...) this might apply also
# but this can be fixed later in extra pull requests from people on the platform
install(FILES argh-config.cmake DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/argh)
install(EXPORT arghTargets DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/argh)
install(FILES "${PackagingTemplatesDir}/argh-config.cmake" DESTINATION ${CMAKE_INSTALL_LIBDIR_ARCHIND}/cmake/argh)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change might require changes in other package managers, that will need to adapt to the new install layout. As long as changes come in a new version, it is doable, but extra effort to implement by other package managers, which logic might become dirtier:

if argh-version < x.y.z:
     files installed in cmake/argh
else
     files installed in arch/cmake/argh

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change might require changes in other package managers, that will need to adapt to the new install layout.

IDK if really. Debian/Ubuntu, Fedora and Arch already support this layout, so it is not "new". IDK about other distros.

@a4z
Copy link
Contributor

a4z commented Jul 26, 2021

SInce my 2 cents where requested, here they are:

I have no idea about using cmake as packaging tool, since I do not know any serious project where such packages are used.
The way to package something is usual: make install DESTDIR=where/ever and put where/ever into the package.
Why now a new subdirectory should be needed for arch for something that is a header only library is not obvious to me and outside of my understanding, so please do not expect any valuable feedback from me to to a topic that I feel is rather not in my domain

@KOLANICH
Copy link
Author

KOLANICH commented Jul 26, 2021

BTW, is there a way to add CI for this?

Surely there is, but it would require quite a lot of work to do it properly.

@adishavit
Copy link
Owner

What's new?

@KOLANICH
Copy link
Author

Nothing new, just rebased.

Fixed installation dir of configs - it must be arch-independent for a header-only library.
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

Successfully merging this pull request may close these issues.

4 participants