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

Add a cpack configuration for library packaging #426

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

Conversation

baptistepetit
Copy link

Following the feature request in issue #361 adding a base CPack configuration allow software packaging out of the box.

@RafalGoslawski
Copy link
Contributor

Hi Baptiste,

Thank you for your contribution.

The resulting .deb and .rpm packages seem to have a few issues when looking at the lintian and rpm-lint output.
lintian.txt
rpm-lint.txt

Can you check if those can be fixed?

I do not have a Windows machine at hand to check if the Windows packages are correct, so it would be nice to confirm if they're correct with some linter as well.

@baptistepetit
Copy link
Author

Hi Rafal,

Thank you for the feedback, I'll check what I can do!

Regarding the NSIS packaging, I am also unable to check the result on a Windows machine, maybe it would be better to drop this option for now and let an interested Windows user develop for it in a separate PR?

@baptistepetit baptistepetit force-pushed the feature/361 branch 3 times, most recently from e11f2dd to feeca3b Compare September 13, 2021 12:21
@baptistepetit
Copy link
Author

Hi again!

I managed to fix a big chunk of the linter complaints!

Most of the remaining complaints are linked to how/where your libraries are being installed, which I am not sure I should touch without more info:

  • A lot of the complaints are about the shared object libamqpcpp.so.4.3 being linked to as libamqpcpp.so while other files (packageconfig, .cmake, and headers) are not being put in a versioned folder at install. Would it be ok for me to change those install location to please the linter?
  • Library is not stripped from debug symbols, that is easy to change adding a -s compiler flag, is it ok for me to do so in this PR?
  • The debian generator wants a changelog file, but what would be the prefered way to integrate one to the project?

@rwols
Copy link
Contributor

rwols commented Sep 13, 2021

Library is not stripped from debug symbols, that is easy to change adding a -s compiler flag, is it ok for me to do so in this PR?

Can't you use:

set(CPACK_STRIP_FILES ON)

?

@baptistepetit
Copy link
Author

Can't you use:
set(CPACK_STRIP_FILES ON)
?

Indeed I can also use this, just wanted to be sure it was ok to strip debug symbols in this project.

Comment on lines +159 to +167
set(CPACK_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS
OWNER_READ
OWNER_WRITE
OWNER_EXECUTE
GROUP_READ
GROUP_EXECUTE
WORLD_READ
WORLD_EXECUTE
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is wise. Why not leave it at the default?

Copy link
Author

Choose a reason for hiding this comment

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

I am actually restricting the rights compared with the automatically generated folders by the install commands in this CMakeLists.txt.
As the generated folders were having too lax permissions lintian was complaining.
I believe it is the same issues that happened here: https://gitlab.kitware.com/cmake/cmake/-/issues/17333

But you are right, the more accurate approach would be to set the folder rights at the file install time.

Copy link
Author

Choose a reason for hiding this comment

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

Update:
After a little research it seems CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS and CPACK_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS are the only way to set permissions for directories implicitly created at install time.
Next Acions
I can change the way to set it to CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS though to be completely identical between install and package target.

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.

3 participants