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

src/cli/CMakeLists.txt: Don't call mandb #2774

Merged
merged 1 commit into from Mar 20, 2019

Conversation

Projects
None yet
4 participants
@Polynomial-C
Copy link
Contributor

commented Mar 8, 2019

There are other man implementations beside man-db so it is not even sure
that the "mandb" binary even exists on all unices. Other than that, usually
there's a cron job running "mandb" on a daily basis.

@louib louib added the plugin: CLI label Mar 8, 2019

@louib

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

@Polynomial-C what about using the ERROR_QUIET option https://cmake.org/cmake/help/v3.12/command/execute_process.html?highlight=error_quiet ??
This way we would still update the mandb on systems where the mandb binary is available.

@Polynomial-C

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

How about cross-compilation? How about build hosts that create .rpm/.deb packages? They all do not require mandb to be run.
In case you desperately wanna keep it can it at least be made optional? Some switch to turn it off/on?

@louib

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

How about cross-compilation? How about build hosts that create .rpm/.deb packages? They all do not require mandb to be run.

I agree. We should only call mandb when installing. How about install(CODE "execute_process(COMMAND mandb -q ERROR_QUIET)")?

@Polynomial-C

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

How does that help for build hosts? They also call install target just with different destination directory so it won't get installed into the running OS.
Again, why even mess with this? Package maintainers can take care of this easily (for example through a post-install hook). A build system should never mess with stuff outside of its purpose which usually is to build software.

@mgorny

This comment has been minimized.

Copy link

commented Mar 20, 2019

All distributions have their own post-install hooks that take care of this. Calling it explicitly is useless in the best case, and will cause fatal sandbox violation in the worst case ('this program tried fiddling with the live system') which will not only cause the updater to fail but terminate the whole build process. So don't do it.

If you really insist on implementing horrible 'we know better than the user' ideas, please move them to a separate target that's not executed by default.

@droidmonkey droidmonkey added this to the v2.4.1 milestone Mar 20, 2019

Don't call mandb
There are other man implementations beside man-db so it is not even sure
that the "mandb" binary even exists on all unices. Other than that, usually
there's a cron job running "mandb" on a daily basis.

@droidmonkey droidmonkey changed the base branch from develop to release/2.4.1 Mar 20, 2019

@droidmonkey

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

@louib we can add the mandb call to the release-tool

@louib

louib approved these changes Mar 20, 2019

@droidmonkey droidmonkey merged commit 4a0bb32 into keepassxreboot:release/2.4.1 Mar 20, 2019

4 checks passed

CodeFactor No issues found.
Details
MacOS (KeepassXC) TeamCity build finished
Details
Ubuntu Linux (KeepassXC) TeamCity build finished
Details
Windows 10 (KeepassXC) TeamCity build finished
Details

droidmonkey added a commit that referenced this pull request Apr 12, 2019

Release 2.4.1
- Fix database deletion when using unsafe saves to a different file system [#2889]
- Fix opening databases with legacy key files that contain '/' [#2872]
- Fix opening database files from the command line [#2919]
- Fix crash when editing master key [#2836]
- Fix multiple issues with apply button behavior [#2947]
- Fix issues on application startup (tab order, --pw-stdin, etc.) [#2830]
- Fix building without WITH_XC_KEESHARE
- Fix reference entry coloring on macOS dark mode [#2984]
- Hide window when performing entry auto-type on macOS [#2969]
- Improve UX of update checker; reduce checks to every 7 days [#2968]
- KeeShare improvements [#2946, #2978, #2824]
- Re-enable Ctrl+C to copy password from search box [#2947]
- Add KeePassXC-Browser integration for Brave browser [#2933]
- SSH Agent: Re-Add keys on database unlock [#2982]
- SSH Agent: Only remove keys on app exit if they are removed on lock [#2985]
- CLI: Add --no-password option [#2708]
- CLI: Improve database extraction to XML [#2698]
- CLI: Don't call mandb on build [#2774]
- CLI: Add debug info [#2714]
- Improve support for Snap theming [#2832]
- Add support for building on Haiku OS [#2859]
- Ctrl+PgDn now goes to the next tab and Ctrl+PgUp to the previous
- Fix compiling on GCC 5 / Xenial [#2990]
- Add .gitrev output to tarball for third-party builds [#2970]
- Add WITH_XC_UPDATECHECK compile flag to toggle the update checker [#2968]
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.