feat: Complete exclude_newer policy - #4344
Conversation
c3bbff2 to
e197fc4
Compare
6ce4cea to
f8c7334
Compare
f8c7334 to
88e0f91
Compare
Klaim
left a comment
There was a problem hiding this comment.
I'll make another review pass later, I didnt reach half of the changes yet.
| * Returns ``std::nullopt`` when parsing fails or trailing characters remain. | ||
| */ | ||
| template <typename T> | ||
| [[nodiscard]] auto parse_chrono(std::string_view value, const char* fmt) -> std::optional<T> |
There was a problem hiding this comment.
Just to be clear: this is necessary only because MacOS's build toolchain doesnt implement std::chrono::parse yet?
If so, could you add a TODO about this in the documentation comments to replace it when std::chrono::parse becomes available everywhere?
There was a problem hiding this comment.
Added a TODO to drop the Howard Hinnant fallback once std::chrono::parse is ubiquitous on supported platforms.
| * | ||
| * See Configurable ``exclude_newer_package``. | ||
| */ | ||
| std::map<std::string, std::string> exclude_newer_package; |
There was a problem hiding this comment.
In the end this is never used as a registry as resolve_exclude_newer_package_cutoffs will just go through elements one by one. I think it would be better if it was a vector of struct, or pair if that helps with CLI11 ?
There was a problem hiding this comment.
Switched Context::exclude_newer_package to std::vector<std::pair<std::string, std::string>>; added YAML convert + dump_json so CLI/env JSON still round-trips as an object.
| namespace mamba | ||
| { | ||
| /** | ||
| * Resolved per-package ``exclude_newer`` cutoffs. |
There was a problem hiding this comment.
kind of side question: why are you using double anti-quote instead of just normal anti-quote for code parts?
If my understanding is correct, markdown is automatically handled by our doxygen setup?
There was a problem hiding this comment.
Updated to single backticks for Doxygen Markdown.
| * | ||
| * The public API exposes ``std::uint64_t`` seconds for compatibility with repodata | ||
| * timestamps. Internally, date and datetime values are parsed with | ||
| * ``std::chrono::parse`` when the standard library provides it, otherwise with |
There was a problem hiding this comment.
I would avoid letting the details of the implementation appear in the description of the interface of a function when it's not a description of the general algorithm being implemented, so that changing the implementation does not make that comment/description obsolete without us noticing.
There was a problem hiding this comment.
Removed implementation details from resolve_exclude_newer_cutoff; kept semantic description only.
| const std::map<std::string, std::string>& exclude_newer_package | ||
| ) -> solver::libsolv::Database::Settings | ||
| { | ||
| const auto now = static_cast<std::uint64_t>(std::time(nullptr)); |
There was a problem hiding this comment.
Why are we not using chrono to get this timepoint value?
There was a problem hiding this comment.
Reference time now uses std::chrono::system_clock.
There was a problem hiding this comment.
(I thought that libc++ did not implemented this part, which is not the case.)
| std::string message; | ||
| if (package_name.empty()) | ||
| { | ||
| message = "Could not parse exclude_newer value '" + std::string(value) + "'; " |
There was a problem hiding this comment.
Consider using fmt instead of string concatenation for generating these messages.
There was a problem hiding this comment.
Parse errors now use fmt::format.
| static const std::regex compact_duration{ R"(^(\d+[yMwdhms])+$)" }; | ||
| static const std::regex compact_segment{ R"((\d+)([yMwdhms]))" }; | ||
|
|
||
| constexpr std::uint64_t seconds_per_minute = 60; |
There was a problem hiding this comment.
This is duplicated from the previous function.
Also I don't think "second per year/month" is correct (doesnt it changes depending on the year?), these values would be better deduced through chrono?
There was a problem hiding this comment.
Deduplicated unit constants; year/month use std::chrono::years/months via duration_cast. Tests updated accordingly.
| return std::nullopt; | ||
| } | ||
|
|
||
| std::uint64_t multiplier = 0; |
There was a problem hiding this comment.
Why not just use the facilities provided by std::chrono instead of doing it manually?
There was a problem hiding this comment.
Compact parser now accumulates std::chrono durations instead of manual multipliers.
e5a8ef2 to
91ba717
Compare
f5b1fbf to
b5a04f0
Compare
Apply clang-format to ternary expression in database.cpp and a pre-existing line-break issue in helpers.cpp.
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Johan Mabille <johan.mabille@gmail.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Klaim <Klaim@users.noreply.github.com>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
b5a04f0 to
1e56c01
Compare
Description
Continues #4228 by completing the
exclude_newerpolicy: wiring config/CLI through to the solver, adding per-package overrides, and hardening duration/date parsing.exclude_newer/exclude_newer_package: global cutoff plus per-package overrides (falseto exempt, or a duration/date/timestamp). Available via RC,CONDA_/MAMBA_env vars, and CLI (--exclude-newer,--exclude-newer-packageas a JSON/YAML dictionary).ExcludeNewerPolicy: resolved cutoffs only (global/per_package); raw config stays onContextand is resolved at solve time viaresolve_exclude_newer_policy.7d,3d12h), ISO 8601 durations (P7D), date-only / datetime values; usesstd::chrono::parsewhen available, otherwise Howard Hinnant'sdate(needed on macOS). Unparseable values throwmamba_error.howardhinnant_datefor macOS static/dev builds; declaremsvcp140_atomic_wait.dllfor Windows micromamba.Background / cross-ecosystem tracking: conda/conda#15759
Type of Change
Checklist
pre-commit run --alllocally in the source folder and confirmed that there are no linter errors.Test plan
test_exclude_newer.cpp, database exclude-newer sections, libmambapyexclude_newer_timestamptests{CONDA,MAMBA}_EXCLUDE_NEWER{,_PACKAGE}andmicromamba install --exclude-newer/--exclude-newer-package … --print-config-only--exclude-newer 7dand a per-package exemption; confirm newer builds are filtered and exempted packages remainhowardhinnant_date-staticand Windows DLL allowlist includesmsvcp140_atomic_wait.dll