Skip to content

external packages: use CPM packages instead of third-party directory#1963

Merged
BareosBot merged 22 commits intobareos:masterfrom
arogge:dev/arogge/master/improve-cpm-integration
Oct 23, 2024
Merged

external packages: use CPM packages instead of third-party directory#1963
BareosBot merged 22 commits intobareos:masterfrom
arogge:dev/arogge/master/improve-cpm-integration

Conversation

@arogge
Copy link
Member

@arogge arogge commented Sep 24, 2024

This is an alternative PR to #1911

  • write a cpm-packages.yaml with a bit of metadata to $CMAKE_BINARY_DIR
  • add a CPM_ONLY-mode to our CMake, so you can run only CPM
  • extend update_license.py to grab the licenses of the CPM packages
  • replace (and update) all third-party dependencies with CPM packages
  • update all pip-tools
  • fix some minor issues (fastlz, truncate systemtest)

TODO:

  • make pr-tool update-license work with CPM licenses
  • clean up duplicates in devtools/template/LICENSE.txt (some CPM licenses already exist, so we'd have duplicates)

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Required backport PRs have been created
  • Correct milestone is set
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@arogge arogge added this to the 24.0.0 milestone Sep 24, 2024
@arogge arogge self-assigned this Sep 24, 2024
@arogge arogge force-pushed the dev/arogge/master/improve-cpm-integration branch 5 times, most recently from 9641e4b to 04f1cc0 Compare October 1, 2024 15:05
@arogge arogge requested a review from pstorz October 8, 2024 10:14
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Looks good to me

@pstorz pstorz self-requested a review October 8, 2024 14:46
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

See comments

@arogge arogge requested a review from pstorz October 11, 2024 09:52
@arogge arogge force-pushed the dev/arogge/master/improve-cpm-integration branch from def2293 to f7cb545 Compare October 22, 2024 11:35
@arogge
Copy link
Member Author

arogge commented Oct 22, 2024

The changes you requested were handled in PR #1977 and #1978.

@pstorz pstorz changed the title use CPM packages instead of third-party directory external packages: use CPM packages instead of third-party directory Oct 23, 2024
bruno-at-bareos and others added 12 commits October 23, 2024 08:34
- remove from third-party
- move USE_SYSTEM_CLI11 to BareosCpmPackages.cmake
- remove set_target_properties IMPORTED_GLOBAL as TRUE by default
  with find/add_librairy
- gtests: remove extra third \n in expected cli help output
  with CLI11 2.4.2 cli --help will output no more extra line

Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
- remove from third-party
- move `USE_SYSTEM_XXHASH` to BareosCpmPackages
- remove set_target_properties IMPORTED_GLOBAL as TRUE by default
  with find/add_library
Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Instead of using CPMLicense.cmake, we now emit a YAML file that contains
a bit of meta-data for the packages that were added via CPM.
This YAML file can then be consumed by pr-tool update-license to
generate a proper LICENSE.txt
Add a new cmake option CPM_ONLY. Setting this to ON will
* add all CPM packages
* stop further configuration

Effectively this will make CPM fetch all packages, using cached packages
when available, and exit. Thus the resulting CMAKE_BINARY_DIR will
contain the CPM packages and the YAML file `cpm-packages.yaml` that
provides a list of the packages along with some meta-information.
The find module for xxHash will now check if building for x86_64,
check if the xxh_x86dispatch.h header is available and set
XXHASH_ENABLE_DISPATCH if both is true.
After carefully re-reviewing xxHash's CMakeLists.txt we can use it.
The offending parts were:
* setting policies
  * these are always set in the CMake versions we require
* overriding CMAKE_BUILD_TYPE
  * mitigated by reordering the includes in our CMakeLists.txt
* overriding BUILD_SHARED_LIBS
  * has no (global) effect

The remaining custom code detects if dispatching is possible and sets
the flag accordingly.
arogge and others added 10 commits October 23, 2024 08:34
adds the infer_license package that can guess what license a given
license-text is.
also update all other pipenv packages
update-license will now fetch the licenses from the CPM packages we
consume.
To achieve this we run `cmake -DCPM_ONLY=ON -DUSE_LOCAL_PACKAGES=OFF`,
consume the resulting cpm-packages.yaml to find the license files and
reformat them into debian copyright format.

On a source tree that doesn't support CPM_ONLY (i.e. Bareos 23 and
older) the old behaviour is retained.
to avoid a duplicate when this license is added using the CPM license
discovery mechanism in the near future, we remove it from the template.
debian 12+ wants to build fully disconnected and all packages we need
are available in the distro.
This patch loosens the software versions CPM requires and adds
dependencies to the Debian control file.
freebsd wants to build fully disconnected and all packages we need
are available in ports.
This patch adds required dependencies to the port's Makefile.
Enable FetchContent and disable CPM_USE_LOCAL_PACKAGES when building
a universal client.
The point of the universal client is to have as few dependencies as
possible. Thus, we should bundle as much as we can, so we don't
accidentally introduce new dependencies.
depending on the version cli11 emits more or less whitespace at the end
of the help output.
By trimming the end of the output, the test now works with different
versions of CLI11.
this adds devtools/install-pip-tools.sh. The script will export the
pip-tools directory of the current commit to ~/.local/lib/bareos-tools,
(re-)create the pipenv environment and put symlinks for the tools into
the first writable directory in your $PATH.
@BareosBot BareosBot force-pushed the dev/arogge/master/improve-cpm-integration branch from d1e465d to e0b83aa Compare October 23, 2024 08:34
@BareosBot BareosBot merged commit 602dbc7 into bareos:master Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants