Skip to content

base: Add VendorChangeManager::ensure_vendor_conf_dir_exists() - #3005

Merged
kontura merged 1 commit into
rpm-software-management:mainfrom
jrohel:ensure_vendor_conf_dir_exists
Oct 27, 2026
Merged

kontura merged 1 commit into
rpm-software-management:mainfrom
jrohel:ensure_vendor_conf_dir_exists

Conversation

@jrohel

@jrohel jrohel commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

save_policy_from_toml()/save_policy_from_compact() write directly into the vendor configuration directory ("/etc/dnf/vendors.d/" or installroot equivalent) and fail if it does not exist. The directory is normally created by the libdnf5 package, but may be missing on a minimal --installroot or a from-source build.

Add ensure_vendor_conf_dir_exists(), which creates the directory (and any missing parents) if needed.

save_policy_from_toml()/save_policy_from_compact() write directly into
the vendor configuration directory (/etc/dnf/vendors.d/ or installroot
equivalent) and fail if it does not exist. The directory is normally
created by the libdnf5 package, but may be missing on a minimal
--installroot or a from-source build.

Add ensure_vendor_conf_dir_exists(), which creates the directory (and
any missing parents) if needed.
@jrohel
jrohel requested a review from a team as a code owner October 26, 2026 07:22
@jrohel
jrohel requested review from kontura and removed request for a team October 26, 2026 07:22
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add vendor configuration directory creation helper

✨ Enhancement 🕐 10-20 Minutes


AI Description

• Add an explicit way to create missing vendor configuration directories before saving policies.
• Resolve the directory against the installroot when host configuration is disabled.
• Report whether creation occurred and raise a vendor manager error on failure.
Diagram

graph TD
  Caller["Policy caller"] --> API["VendorChangeManager API"] --> Impl["Manager implementation"] --> Path["Vendor path resolution"] --> Create["Create directories"] --> Dir["Vendor config directory"]
  Create -. "failure" .-> Error["Manager error"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Create the directory within save operations
  • ➕ Callers would not need to remember a preparation step.
  • ➖ Changes existing save behavior and introduces filesystem side effects into every save attempt.

Recommendation: The explicit helper fits the stated need while preserving existing save semantics. Callers must invoke it before saving in environments where the directory may be absent; the save methods themselves remain unchanged.

Files changed (2) +32 / -0

Enhancement (2) +32 / -0
vendor_change_manager.hppExpose directory preparation on VendorChangeManager +8/-0

Expose directory preparation on VendorChangeManager

• Declares and documents an explicit helper for creating the vendor configuration directory and missing parents. Specifies its creation-status return value and failure exception.

include/libdnf5/base/vendor_change_manager.hpp

vendor_change_manager.cppImplement vendor directory creation +24/-0

Implement vendor directory creation

• Resolves the existing host or installroot vendor path and calls std::filesystem::create_directories. Returns whether creation occurred and wraps filesystem errors in VendorChangeManagerError.

libdnf5/base/vendor_change_manager.cpp

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review
Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗


Powered by Qodo

@kontura kontura left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can I ask why is this a separate new API?
I was just thinking that if the vendor conf dir was automatically ensured on each save_policy_from_toml()/save_policy_from_compact() the API would be a bit more user friendly.

@kontura kontura self-assigned this Oct 27, 2026
@jrohel

jrohel commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Good question.

In config-manager, missing directories are only created when the user explicitly passes --create-missing-dir. Exposing this through a separate API method keeps the behavior explicit and consistent, allowing the CLI (or any other caller) to opt in. My current vendor PR #2935 in config-manager doesn't create directories at all right now, but with this API method, I can add --create-missing-dir there too for full consistency.

On the other hand, my PR #3004 for the copr plugin automatically allows creating missing directories using this new API. The copr plugin already displays prominent warnings about the security risks of enabling third-party repositories. Once the user explicitly confirms and accepts that risk, bothering them with an extra prompt just for a creating missing directory would only degrade the UX.

@kontura
kontura added this pull request to the merge queue Oct 27, 2026
Merged via the queue into rpm-software-management:main with commit 4f20219 Oct 27, 2026
17 of 18 checks passed
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.

2 participants