Skip to content

Commit

Permalink
CMake does now install cfg files.
Browse files Browse the repository at this point in the history
  • Loading branch information
@guy-maurel
guy-maurel committed May 14, 2019
1 parent 72de5fd commit ea69500
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CMakeLists.txt
View file
Expand Up @@ -459,6 +459,10 @@ else()
install(FILES "${CMAKE_CURRENT_BINARY_DIR}/uncrustify.1"
DESTINATION "${CMAKE_INSTALL_MANDIR}/man1"
)
install(DIRECTORY "${PROJECT_SOURCE_DIR}/etc/"
DESTINATION "${CMAKE_INSTALL_DATAROOTDIR}/uncrustify"
FILES_MATCHING PATTERN "*.cfg"
)
endif()

#
Expand Down

5 comments on commit ea69500

@AMDmi3
Copy link
Contributor

@AMDmi3 AMDmi3 commented on ea69500 Nov 18, 2019

Choose a reason for hiding this comment

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

Are you sure this is correct? Before, I've installed these manually to etc/uncrustify, and IMO that's more correct place for them. The difference is that etc is meant for users to look into and/or add their own files (this case), while share is generally meant only to software to look into, and should not be modified by the user.

As a result, if configs are not installed into etc, user would not even know of them unless he uses packaging tool to list all files installed by uncrustify package.

I suggest to either change it to etc/uncrustify, or to allow -c option to take a config name in addition to full path + implement another option to list available configs.

@mwoehlke-kitware
Copy link
Collaborator

Choose a reason for hiding this comment

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

That... seems like a plausible argument: use CMAKE_INSTALL_SYSCONFDIR instead of CMAKE_INSTALL_DATAROOTDIR?

@AMDmi3
Copy link
Contributor

@AMDmi3 AMDmi3 commented on ea69500 Nov 20, 2019

Choose a reason for hiding this comment

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

use CMAKE_INSTALL_SYSCONFDIR instead of CMAKE_INSTALL_DATAROOTDIR?

Yes, this works

@mcatanzaro
Copy link

Choose a reason for hiding this comment

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

Does uncrustify read these configuration files itself? And are users expected to edit them? Or are they just intended as examples?

The manpage says:

   Basic Options:
       -c CFG Use the config file CFG.
              If  not  specified,  uncrustify  will  use $UNCRUSTIFY_CONFIG or
              $HOME/.uncrustify.cfg.

which seems to imply these files are only examples, not actually used unless the user manually selects them using -c? If so, then I think the current location under /usr/share is best. Putting them under /etc would be appropriate if editing them were to change the behavior of uncrustify. Yes?

@AMDmi3
Copy link
Contributor

@AMDmi3 AMDmi3 commented on ea69500 Nov 27, 2019

Choose a reason for hiding this comment

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

Makes sense as long as you imply EXAMPLESDIR (e.g. ${PREFIX}/share/examples/uncrustify) and not DATADIR. Created #2560

Please sign in to comment.