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

Allow using system libfmt #3278

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Allow using system libfmt #3278

wants to merge 2 commits into from

Conversation

jeandudey
Copy link

Some distributions such as GNU Guix don't allow using bundled dependencies and code, so I'm adding the option to use a system version of libfmt instead of using the bundled one.

Had to nest fmt and CL in another sub-directory of the same name to avoid accidental inclusion of the fmt headers.

@SChernykh
Copy link
Contributor

don't allow using bundled dependencies and code

How do they not allow it? It just doesn't compile? What is the error?

@jeandudey
Copy link
Author

How do they not allow it? It just doesn't compile? What is the error?

@SChernykh the GNU Guix project generally tries to remove bundled dependencies from packages if it's available already, for example, fmt is already packaged and it the library tests are run after building it, libraries are also un-bundled for auditability and to avoid re-building code that's already built.

For example, on the package definition the hwloc library is unbundled as it is available on GNU Guix and xmrig supports building without it being on the src/3rdparty directory, other libraries such as OpenCL headers and libfmt are also available but require changes in xmrig first to avoid patching large chunks of the source code in the package definition.

This is done so that GNU Guix creates a source archive of the xmrig source code but doesn't contain bundled code in the src/3rdparty directory.

@SChernykh
Copy link
Contributor

SChernykh commented May 29, 2023

In this case it should have CMakeLists option for libfmt, just like hwloc has WITH_HWLOC. Also, the PR should be made to dev branch, not master.

Edit: this option should be called something like WITH_BUNDLED_FMT and default to ON because it doesn't just turn off fmt.

@jeandudey
Copy link
Author

jeandudey commented May 29, 2023

Added the option in f0f6d16.

(made fixup commit to squash once the review is done)

Signed-off-by: Jean-Pierre De Jesus DIAZ <me@jeandudey.tech>
@jeandudey jeandudey changed the base branch from master to dev May 29, 2023 18:16
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.

None yet

2 participants