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

Add support for repos configuration overrides #820

Merged

Conversation

jrohel
Copy link
Contributor

@jrohel jrohel commented Aug 15, 2023

After the repositories configuration is loaded, new code is run that loads the override files from the /usr/share/dnf5/repos.override.d and /etc/dnf/repos.overide.d directories.

The format of override files is the same as regular ".repo" files. However, override files can modify the settings of already existing repositories. They cannot create new repositories.
Overide files support glob in the repository ID (in the configuration file section name) to support bulk modification of repositories parameters.

The order of processing the files with overrides:

  1. Creates an alphabetically sorted list of all filenames with overrides in the distribution override directory ("/usr/share/dnf5/repos.override.d") and the user override directory ("/etc/dnf/repos.overide.d"). If a file with the same name is present in both override directories, only the file from the user override directory is added to the sorted list. So, the distribution override file can be simply masked by creating a file with the same name in the user override directory.
  2. Retrieves the overrides from the files in the sorted list. Follows the order. The override from the next file overrides the previous one - the last override value wins.

The PR also includes a commit that adds support for the ']' character in the section header name in IniParser. Allows to use more complex glob patterns for repository IDs.


Example of the repos configuration override file:

# Enable `skip_if_unavailable` for all repositories
[*]
skip_if_unavailable = true

# And then disable `skip_if_unavailable` for repositories with id prefix "fedora"
[fedora*]
skip_if_unavailable = false

An example showing the order in which override files are processed:

Files with user repos overrides:
/etc/dnf/repos.overide.d/20-user-overrides.repo
/etc/dnf/repos.overide.d/60-something2.repo
/etc/dnf/repos.overide.d/80-user-overrides.repo
/etc/dnf/repos.overide.d/99-config-manager.repo

Files with distribution repos overrides:
/usr/share/dnf5/repos.overide.d/50-something2.repo
/usr/share/dnf5/repos.overide.d/60-something2.repo
/usr/share/dnf5/repos.overide.d/90-something2.repo

Resulting file processing order:

  1. /etc/dnf/repos.overide.d/20-user-overrides.repo
  2. /usr/share/dnf5/repos.overide.d/50-something2.repo
  3. /etc/dnf/repos.overide.d/60-something2.repo
  4. /etc/dnf/repos.overide.d/80-user-overrides.repo
  5. /usr/share/dnf5/repos.overide.d/90-something2.repo
  6. /etc/dnf/repos.overide.d/99-config-manager.repo

@jrohel jrohel force-pushed the feature/conf_repos_overrides branch 2 times, most recently from fede15a to b18dd05 Compare August 16, 2023 06:24
@jrohel jrohel force-pushed the feature/conf_repos_overrides branch from b18dd05 to b691c56 Compare August 25, 2023 10:36
@jrohel
Copy link
Contributor Author

jrohel commented Aug 28, 2023

I added CI tests rpm-software-management/ci-dnf-stack#1368

@Conan-Kudo
Copy link
Member

We need to be able to move packaged repos out of /etc too, but this is a good start.

@jrohel jrohel force-pushed the feature/conf_repos_overrides branch from 72cb511 to d219818 Compare September 1, 2023 14:49
@jrohel
Copy link
Contributor Author

jrohel commented Sep 1, 2023

I rebased the code to resolve conflicts and deduplicate create_sorted_file_list function.

libdnf5/utils/iniparser.cpp Outdated Show resolved Hide resolved
@ppisar
Copy link
Contributor

ppisar commented Sep 1, 2023

Otherwise it looks good for my eye untrained for C++.

@jrohel jrohel force-pushed the feature/conf_repos_overrides branch from d219818 to 1ff5a3a Compare September 4, 2023 05:14
Copy link
Contributor

@ppisar ppisar left a comment

Choose a reason for hiding this comment

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

I don't see any mistake. Yet I did not test the code.

@j-mracek j-mracek self-assigned this Sep 11, 2023
void create_repos_from_system_configuration();

/// Loads repositories configuration overrides from drop-in directories. No new repositories are created.
/// Only the configuration of the coresponding existing repositories is modified.
void load_repos_configuration_overrides();
Copy link
Contributor

Choose a reason for hiding this comment

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

What about to make it private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Good idea.

@@ -38,45 +39,6 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

namespace fs = std::filesystem;

namespace {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not 100% sure but the commit might be merged with previous commit or is there any reason why you would prefer the split?


// Returns the input string with the undoubled characters ']'.
// A sequence of two chars ']' is returned as a single char ']'.
std::string section_name_decode_bracket(const std::string & section_name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it is required for glob support in section header like [fedora[2-8]]. @jrohel Is it correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about to use the sema logic like we have in NEVRA parser - when the second [ then the parser skips everything until the next ]. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@j-mracek OK. I changed the logic to be compatible with the NEVRA parser.

After the repositories configuration is loaded, new code is run that loads
the override files from the "/usr/share/dnf5/repos.override.d"
and "/etc/dnf/repos.overide.d" directories.

The format of override files is the same as regular ".repo" files.
However, override files can modify the settings of already existing
repositories. They cannot create new repositories.
Overide files support glob in the repository ID (in the configuration file
section name) to support bulk modification of repositories parameters.

The order of processing the files with overrides:
1. Creates an alphabetically sorted list of all filenames with overrides
   in the distribution override directory ("/usr/share/dnf5/repos.override.d")
   and the user override directory ("/etc/dnf/repos.overide.d"). If a file
   with the same name is present in both override directories, only the file
   from the user override directory is added to the sorted list.
   So, the distribution override file can be simply masked by creating a file
   with the same name in the user override directory.
2. Retrieves the overrides from the files in the sorted list. Follows
   the order. The override from the next file overrides the previous one
   - the last override value wins.

-------------------------------------------------
Example of the repos configuration override file:

[*]
skip_if_unavailable = true

[fedora*]
skip_if_unavailable = false

-------------------------------------------------------------------
An example showing the order in which override files are processed:

Files with user repos overrides:
/etc/dnf/repos.overide.d/20-user-overrides.repo
/etc/dnf/repos.overide.d/60-something2.repo
/etc/dnf/repos.overide.d/80-user-overrides.repo
/etc/dnf/repos.overide.d/99-config-manager.repo

Files with distribution repos overrides:
/usr/share/dnf5/repos.overide.d/50-something2.repo
/usr/share/dnf5/repos.overide.d/60-something2.repo
/usr/share/dnf5/repos.overide.d/90-something2.repo

Resulting file processing order:
1. /etc/dnf/repos.overide.d/20-user-overrides.repo
2. /usr/share/dnf5/repos.overide.d/50-something2.repo
3. /etc/dnf/repos.overide.d/60-something2.repo
4. /etc/dnf/repos.overide.d/80-user-overrides.repo
5. /usr/share/dnf5/repos.overide.d/90-something2.repo
6. /etc/dnf/repos.overide.d/99-config-manager.repo
Older changes (Use utils::fs::File instead of streams) caused part of
the code to become unreachable (dead).
The section header name is between '[', ']' characters. So the ']'
character is interpreted as the end of the section name. This patch adds
support for the ']' character in the section name.
Doubling the character ']' - the sequence "]]" - means inserting
a single character ']' into the section name.
Some lines in the raw strings ended with spaces. And some editors remove
trailing spaces during code reformatting.
Using a normal string is more robust.
@j-mracek
Copy link
Contributor

There is one problem - PR does not contain documentation, but it can be delivered later, but it requires to create an issue.

@Conan-Kudo
Copy link
Member

There is one problem - PR does not contain documentation, but it can be delivered later, but it requires to create an issue.

I've created an issue to track this as #883

@j-mracek
Copy link
Contributor

LGTM

@j-mracek j-mracek added this pull request to the merge queue Sep 11, 2023
Merged via the queue into rpm-software-management:main with commit 1ce386d Sep 11, 2023
7 checks passed
@jrohel jrohel mentioned this pull request Nov 7, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants