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

Noble apt RepositoryMapping unsupported #135

Closed
yanksyoon opened this issue Nov 14, 2024 · 5 comments · Fixed by #137
Closed

Noble apt RepositoryMapping unsupported #135

yanksyoon opened this issue Nov 14, 2024 · 5 comments · Fixed by #137
Assignees

Comments

@yanksyoon
Copy link

The RepositoryMapping object in apt lib only reads from "/etc/apt/sources.list" and "/etc/apt/sources.list.d/*.list".

Noble's sources are located at /etc/apt/sources.list.d/ubuntu.sources, hence raising InvalidSourceError.

@dimaqq
Copy link

dimaqq commented Nov 14, 2024

A PR would be very welcome :)

@yanksyoon
Copy link
Author

@dimaqq out of curiosity, may I ask what team is currently maintaining this repository please?

@benhoyt
Copy link
Collaborator

benhoyt commented Nov 14, 2024

Our team has picked up maintenance of this repo. We don't have a ton of cycles to spend on maintenance, but this particular one seems like a significant issue! @james-garner-canonical, getting this charm lib to work on Noble seems like a noble task, so assigning to you -- perhaps you can take a look at this in the next week or two.

@NucciTheBoss
Copy link
Contributor

@james-garner-canonical have you had chance to tackle this issue yet? I'm getting ganked by it as the HPC team is upgrading the Slurm charms to Noble so that we can have cool kid dkms modules and fancy new login node daemons.

Looking at the required changes, we'll likely need to rework how the sources lists are constructed as Noble uses the deb822 standard, which the apt charm library doesn't support; it only supports the one-line style format. The extensions are separated between .list and .sources respectively, so that could be used as the indicator for which parser should be used.

The parser itself seems pretty simple itself by just iterating the over sources list line by line. deb822 might be more interesting since the sources are specified in blocks rather than lines.

@james-garner-canonical
Copy link
Contributor

Thanks for raising this again @NucciTheBoss, and for the helpful info about the deb822 standard. I'll look into this today

james-garner-canonical added a commit that referenced this issue Dec 5, 2024
Ubuntu 24.04 uses [deb288 format](https://manpages.ubuntu.com/manpages/noble/en/man5/sources.list.5.html) sources, which python-libjuju does not currently support (as pointed out by @yanksyoon and @NucciTheBoss in #135). This merge commit adds support for reading `*sources` files.

`RepositoryMapping` now automatically findes them in `/etc/apt/sources.list.d` when initialised. Additionally, to avoid an `InvalidSourceError` if `/etc/apt/sources.list` exists but only includes a comment about the new sources location (as it does on noble), we ignore this error if `/etc/apt/sources.list.d/ubuntu.sources` exists.

This merge commit also cleans up the `add` method by calling `add-apt-repository` internally instead of writing directly to a file. Note that this means that adding a disabled repository no longer has any effect (previously it would write a a file containing a single, commented-out one-line repository definition).

Original commit messages below.

---

* feat: support deb822 style source specification

Ubuntu 24.04 adopts the deb822 style source specification. Such files
are listed in /etc/apt/sources.list.d/*sources, and allow the
specification of sources in a multi-line format

* refactor: separate file reading and object updating from parsing lines

* refactor: preserve line nos when parsing deb822 lines into paragraphs

* refactor: move turning numbered lines into options dict to helper

* feat: support inline comments in deb822 source files

* tests: add some unit tests for _iter_deb822_paragraphs

* fix: raise an InvalidSourceError on missing required deb822 keys

* tests: add some unit tests for _get_deb822_options

* fix: correctly handle newline terminated lines in paragraph iteration

* tests: add some unit tests for _iter_deb822_paragraphs

* tests: add some tests for _parse_deb822_lines

* feat: add the number of errors to the debug output in load_deb822

* tests: add some unit tests for load_deb822

* tests: add a unit test for initialiasing RepositoryMapping with deb822

* style: tox -e fmt tests/unit/test_apt.py

* fix: correct docstring

* fix: python3.8 context managers and clean up linter directives

* feat: make DebianRepository aware of deb822 format for some operations

add: add ability to write a deb822 format file
disable: raise NotImplementedError for deb822 format files
gpg_key: use existing import_key functionality to provide keys specified
    in the stanza itself as a file for compatibility
Also refactor Deb822 functionality to a separate class.
Also move deb822 unit tests to the more appropriate test_repo.py

* fix: remove extraneous information from docstring

* feat: use apt-add-repository for RepositoryMapping.add

* refactor: logic and testing cleanup

* style: type annotate integration test helper function

* feat: support apt autoremove in remove_package

* feat: debug useful information on error in apt.update

* feat: log when writing to a sources file in from_repo_line

* feat: make the apt directories (private) attributes (for testing)

* style: correct RepositoryMapping.__iter__ annotation and add FIXME

* feat: log the filename as well as the number of repos when parsing

* feat: refactor RepositoryMapping.add to possible support remove as well

* feat: allow _Deb822Stanza from an empty set of lines

* tests: add unit tests that use files on disk

* tests: update integration tests

* feat: use sourceslist (one-per-line) format for add-apt-repository

* feat: call _add_apt_repository in from_repo_line (don't reimplement logic)

* tests: re-enable package cleanup in integration tests

* tests: add test case from hardware-observer charm and cleanup keys

* tests: refactor deb822 unit tests

* tests: iterate on deb822 tests

* tests: fully refactor and expand deb822 unit tests

* style: make linting happy

* test: fix an erroneous assert in integration tests

* refactor: clean up diff, signatures, etc

* refactor: move keys to files

* fix: don't warn about key file on remove

* style: use tuple for endswith

* style: paragraph -> stanza, repositories -> repos

* test: switch from inkscape to terminator ppa for ci

* test: switch from terminator to fish ppa for ci

* chore: remove ValueError from RepositoryMapping.add

* test: remove old tests from test_repo.py
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 a pull request may close this issue.

5 participants