Skip to content

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Jan 16, 2024

The package gnu-free-fonts does not exist, as seen for example in

gnu-free-fonts is merely the name of the source package, the actual rpms are called gnu-free-mono-fonts, gnu-free-sans-fonts , gnu-free-serif-fonts https://koji.fedoraproject.org/koji/buildinfo?buildID=2308657

Here we adjust for it.

According to https://rpmfind.net/linux/rpm2html/search.php?query=gnu-free-sans-fonts, the packages are available for fedora >=37, EPEL 9, centos-7, centos-stream-8. Accordingly we remove the setting IGNORE_MISSING_SYSTEM_PACKAGES=no from fedora < 37.

Also adding fedora-40, the new rawhide.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 16, 2024

Actually, it looks like gnu-free-fonts is merely the name of the source package - https://src.fedoraproject.org/rpms/gnu-free-fonts

The actual rpms are called gnu-free-mono-fonts, gnu-free-sans-fonts , gnu-free-serif-fonts https://koji.fedoraproject.org/koji/buildinfo?buildID=2308657

@dimpase
Copy link
Member

dimpase commented Jan 19, 2024

OK for Fedora 38 and OpenBSD - tested on real systems

Copy link

github-actions bot commented Feb 3, 2024

Documentation preview for this PR (built with commit b7032a1; changes) is ready! 🎉

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 6, 2024

Thanks for fixing this. LGTM.

Did this pass the "CI Linux Incremental" test? Perhaps "CI Linux Incremental" does not check optional system packages.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 6, 2024

I tested with some fedora versions. It works well.

I read this:

    # Set this to 'yes' instead of 'no' to ignore missing system packages - by installing them one by one
    # and ignoring errors.  We use that to take care of old versions of distributions.
    # For -maximal environments, the default is 'yes' but later we override it for rolling distributions
    # (but not for unstable distributions that often have intermittent issues).
                                               IGNORE_MISSING_SYSTEM_PACKAGES=no
    maximal:                                   IGNORE_MISSING_SYSTEM_PACKAGES=yes

but I still don't understand the purpose of IGNORE_MISSING_SYSTEM_PACKAGE. So would you explain this statement

According to https://rpmfind.net/linux/rpm2html/search.php?query=gnu-free-sans-fonts, the packages are available for fedora >=37, EPEL 9, centos-7, centos-stream-8. Accordingly we remove the setting IGNORE_MISSING_SYSTEM_PACKAGES=no from fedora < 37.

?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 6, 2024

I tested with some fedora versions. It works well.

Thanks for testing!

I read this:

    # Set this to 'yes' instead of 'no' to ignore missing system packages - by installing them one by one
    # and ignoring errors.  We use that to take care of old versions of distributions.
    # For -maximal environments, the default is 'yes' but later we override it for rolling distributions
    # (but not for unstable distributions that often have intermittent issues).
                                               IGNORE_MISSING_SYSTEM_PACKAGES=no
    maximal:                                   IGNORE_MISSING_SYSTEM_PACKAGES=yes

but I still don't understand the purpose of IGNORE_MISSING_SYSTEM_PACKAGES.

It's all about balancing the effective use of system packages vs. the burden of maintenance:

  • If system packages that we refer to are renamed or become unavailable otherwise, we want the CI to tell us; we don't want to find out from user complaints. So for standard packages on reasonable versions of distributions, we make it a hard error if the listed system packages cannot be installed.
  • To avoid a much larger burden of maintenance, we only have one set of distro package information for each large family of distributions. All of fedora, centos, rhel is recorded in fedora.txt. All of debian, ubuntu, and the various value-added or sugar-added variants is recorded in debian.txt etc. So if a variant misses one of our standard packages – tough luck, we'll just set IGNORE_MISSING_SYSTEM_PACKAGES=yes for this variant. This means that when testing that variant, we determine the available subset of packages dynamically. When a package disappears, we can see it in the log, but the test of the platform is still supposed to go through.
  • Testing the -maximal package configuration has two main purposes: 1) To test that system packages that we don't know how to use (no spkg-configure.m4) don't break the build by their sheer presence on the system. 2) For tests of optional packages that depend on other optional packages.

So would you explain this statement

According to https://rpmfind.net/linux/rpm2html/search.php?query=gnu-free-sans-fonts, the packages are available for fedora >=37, EPEL 9, centos-7, centos-stream-8. Accordingly we remove the setting IGNORE_MISSING_SYSTEM_PACKAGES=no from fedora < 37.

?

Before this PR, we had the line

fedora-34:                                 IGNORE_MISSING_SYSTEM_PACKAGES=no

This marked fedora-34 as a very reasonable distro variant: We require that all system packages listed in fedora.txt, whether standard or optional, whether we know how to use them or not, must be installable. And running the fedora-34-maximal tested that, giving a hard error otherwise.

The comment from the ticket description says that because gnu-free-sans-fonts is not available on fedora-34, we should no longer demand that fedora-34-maximal has all listed packages. By removing the line setting IGNORE_MISSING_SYSTEM_PACKAGES=no, we are back to using the default: IGNORE_MISSING_SYSTEM_PACKAGES=no for -minimal and -standard, but IGNORE_MISSING_SYSTEM_PACKAGES=yes for -maximal.

It's admittedly a bit twisty...

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 7, 2024

Thanks for the detailed explanation.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 7, 2024

Thanks!

vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 7, 2024
sagemathgh-37073: `build/pkgs/free_fonts`: Fix fedora system package information
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

The package `gnu-free-fonts` does not exist, as seen for example in
- `fedora-36-maximal` (https://github.com/sagemath/sage/actions/runs/752
1668027/job/20497322866).
- `fedora-39-maximal` (https://github.com/sagemath/sage/actions/runs/752
1668027/job/20497323960#step:11:20270)

`gnu-free-fonts` is merely the name of the source package, the actual
rpms are called `gnu-free-mono-fonts`, `gnu-free-sans-fonts `, `gnu-
free-serif-fonts`
https://koji.fedoraproject.org/koji/buildinfo?buildID=2308657

Here we adjust for it.

According to https://rpmfind.net/linux/rpm2html/search.php?query=gnu-
free-sans-fonts, the packages are available for fedora >=37, EPEL 9,
centos-7, centos-stream-8. Accordingly we remove the setting
`IGNORE_MISSING_SYSTEM_PACKAGES=no` from fedora < 37.

Also adding fedora-40, the new rawhide.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37073
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
@vbraun vbraun merged commit ed119f6 into sagemath:develop Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants