Skip to content

maint: Make spdlog a private dependency #3945

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

maint: Make spdlog a private dependency #3945

wants to merge 1 commit into from

Conversation

jjerphan
Copy link
Member

Fix #3925.

⚠️ The run-export on spdlog will need to be removed then.

@jjerphan jjerphan added the release::maintenance For PRs related to maintenance label May 19, 2025
@jjerphan jjerphan marked this pull request as ready for review May 19, 2025 08:17
@codecov Codecov
Copy link

codecov bot commented May 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@aaae25f). Learn more about missing BASE report.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #3945   +/-   ##
=======================================
  Coverage        ?   63.73%           
=======================================
  Files           ?      302           
  Lines           ?    37912           
  Branches        ?     2836           
=======================================
  Hits            ?    24165           
  Misses          ?    13692           
  Partials        ?       55

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Hind-M
Copy link
Member

Hind-M commented May 19, 2025

Not sure if this is enough or if we are missing other things, but we should at least remove spdlog includes from exposed headers (e.g https://github.com/mamba-org/mamba/blob/main/libmamba/include/mamba/core/util_scope.hpp#L9 )?

JohanMabille
JohanMabille requested changes May 20, 2025
View reviewed changes
Copy link
Member

@JohanMabille JohanMabille left a comment •
edited
Loading

Choose a reason for hiding this comment

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

spdlog cannot be a private dependency as long as spdlog.h is included (even indirectly) in a publicly exposed header of libmamba.

I think it should not be problematic to make util_scope.hpp (the only eader including it directly) a private header of libmamba,; the issue is that the libmamba tests also use it; adding the src folder to the include directories of test_libmamba would solve the issue for local build, but it would not work for testing already installed libmamba (and I think we probably need this use case for some package managers). Notice that this code could be duplicated for the tests though.

@JohanMabille
Copy link
Member

JohanMabille commented May 24, 2025 •
edited
Loading

Actually let's go with private headers, and deactivate the tests of private headers in the context of an already installed library. The libmamba test CMakeLists.txt already adds the source directories of mamba to the include directories of the test target.

Klaim added a commit to Klaim/mamba that referenced this pull request Jul 9, 2025
@Klaim
Customizable libmamba logging implementation
f4e4de5 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Jul 9, 2025
@Klaim
Customizable libmamba logging implementation
e51333a 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
@Klaim Klaim mentioned this pull request Jul 16, 2025
Open
19 tasks
Klaim added a commit to Klaim/mamba that referenced this pull request Jul 29, 2025
@Klaim
Customizable libmamba logging implementation
96917a0 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Jul 31, 2025
@Klaim
Customizable libmamba logging implementation
11641d3 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Aug 8, 2025
@Klaim
Customizable libmamba logging implementation
c323c34 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Aug 12, 2025
@Klaim
Customizable libmamba logging implementation
d2cba46 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Aug 12, 2025
@Klaim
Customizable libmamba logging implementation
d8962de 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Sep 2, 2025
@Klaim
Customizable libmamba logging implementation
1b21dc8 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Sep 3, 2025
@Klaim
Customizable libmamba logging implementation
5eff10e 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Sep 8, 2025
@Klaim
Customizable libmamba logging implementation
1d7e3b1 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Sep 23, 2025
@Klaim
Customizable libmamba logging implementation
e1b55a4 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Sep 30, 2025
@Klaim
Customizable libmamba logging implementation
cee5adf 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Oct 9, 2025
@Klaim
Customizable libmamba logging implementation
abb3744 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Oct 14, 2025
@Klaim
Customizable libmamba logging implementation
08218d4 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Oct 14, 2025
@Klaim
Customizable libmamba logging implementation
b603c85 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Klaim added a commit to Klaim/mamba that referenced this pull request Oct 17, 2025
@Klaim
Customizable libmamba logging implementation
1c2b064 
- new logging system for users to plug-in a log handler which will receive
  log records from `libmamba` and do whatever appropriate logging impl
  they want with it (this is separate from the "console" output, only
  concerns logging);
- provides a `spdlog`-based log handler, which should do what previous
  `libmamba` versions did;
- `Context` can be initialized with a provided log handler;
- By default, if no provided log handler and logging is enabled,
  `Context` will setup a `spdlog`-based log handler;

(more to come: stdlib log handler, noop log handler, test log handler)

Note that this is a stepping stone towards removing `spdlog` as dependency
of `libmamba` (see also mamba-org#3945)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohanMabille JohanMabille JohanMabille requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

release::maintenance For PRs related to maintenance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

spdlog should be a private dependency of libmamba

3 participants

@jjerphan @Hind-M @JohanMabille