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

Fix failing on EPEL check in Fedora #111

Merged
merged 2 commits into from
Apr 19, 2023
Merged

Conversation

samh
Copy link
Contributor

@samh samh commented Feb 25, 2023

The "package" install method was failing on Fedora, because it is part of the 'RedHat' OS family but does not have EPEL.

Only applies when using the "package" install method. Added a separate variable "borg_require_epel" to make it easy to disable the check in any other situations (e.g. using a custom mirror instead of the epel-release package).

@m3nu
Copy link
Collaborator

m3nu commented Mar 6, 2023

There is a big PR open right now. May integrate this PR after the bigger one to avoid conflicts.

Only applies when using the "package" install method.
Added a separate variable "borg_require_epel" to make it easy to disable
the check in any other situations (e.g. using a custom mirror instead of
the epel-release package).
@samh samh force-pushed the fedora-package-fix branch from c833ca2 to 7876794 Compare March 30, 2023 21:28
@samh
Copy link
Contributor Author

samh commented Mar 30, 2023

I rebased it and added the variable to the README.

README.md Outdated
@@ -87,6 +87,7 @@ $ git clone https://github.com/borgbase/ansible-role-borgbackup.git roles/ansibl
- `borg_exclude_from`: Read exclude patterns from one or more separate named files, one pattern per line.
- `borg_exclude_patterns`: Paths or patterns to exclude from backup. See [official documentation](https://borgbackup.readthedocs.io/en/stable/usage/help.html#borg-help-patterns) for more.
- `borg_install_method`: By default `pip` is used to install borgmatic. To install via your distributions package manager set this to `package` and (if needed) overwrite the `borg_distro_packages` variable to contain your distributions package names required to install borgmatic. Note that many distributions ship outdated versions of borgbackup and borgmatic; use at your own risk.
- `borg_require_epel`: When using `borg_install_method: package` on RHEL-based distributions, the EPEL repo is required. To disable the check (e.g. when using a custom mirror instead of the epel-release package), set this to `false`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about explaining the default value? Like "Defaults to true on Enterprise Linux-based distros, except Fedora."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, thanks, I will add something - let me think about the wording. I don't think Fedora is an "Enterprise Linux-based distro" (it's the other way around, EL is based on Fedora), so maybe just "Defaults to true on Enterprise Linux-based distros"? Or something about the RedHat OS family except Fedora. I wish there was a better Ansible Fact for EL distros, I was kind of shocked when I noticed Fedora was in there.

@m3nu
Copy link
Collaborator

m3nu commented Mar 31, 2023

Looks good. Just added one comment to make sure the default value is clear enough?

@m3nu m3nu merged commit 7efeb1b into borgbase:master Apr 19, 2023
@m3nu
Copy link
Collaborator

m3nu commented Apr 19, 2023

Thanks for the contribution!

Fixes #124

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