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

Long-term maintainability goals #234

Open
1 of 4 tasks
whitequark opened this issue Nov 14, 2020 · 2 comments
Open
1 of 4 tasks

Long-term maintainability goals #234

whitequark opened this issue Nov 14, 2020 · 2 comments
Labels
gateware Component: gateware software Component: software

Comments

@whitequark
Copy link
Member

whitequark commented Nov 14, 2020

Please DO NOT start working on tasks from this list on your own. These tasks are very complex, have far-reaching consequences, and require subtle API design. In most cases, I (@whitequark) have a fairly specific solution in mind, which I will eventually transfer to a dedicated issue for collaborating. Unless you discuss any work on these tasks beforehand, it is unlikely to be accepted.

  • 1. Replace argparse with a new, almost certainly in-house argument parsing system.

    Although argparse has served us extremely well so far (it is quite impressive how well it supports Glasgow's sprawling, complicated CLI), there are many issues with it:

    • rough at edges (e.g. Don't rely on argparse's internal API #204),
    • doesn't integrate well with our very specific domain (add_pin_argument is the worst offender),
    • requires significant amounts of imperative glue code in applets,
    • is not based on an abstract model that could be shared between CLI and GUI,
    • does not allow an inheriting applet a free choice between keeping its ancestor's options in the CLI or setting a fixed value for them (to a degree this is mitigated with highly modular subtargets, but the overhead is high for something like program-ice40-sram);
    • does not easily admit formatted help text (cli.TextHelpFormatter is an abomination, Markdown or reST would be ideal),
    • does not easily support multiple applets (Multiple Simultaneous Applets #180),
    • does not allow improving autogenerated help text for choice options (e.g. no way to show only non-preview applets, or non-contrib ones, etc),
    • is inherently unable to support self-chainable applets (e.g. JTAG-over-BSCAN-over-JTAG-...) because it requires eagerly defining all subparsers and has only a single namespace for every argument/option,
    • does not allow automatically renaming conflicting options to disambiguate (e.g. in cases of applet inheritance).

    A particularly unpleasant consequence of the imperative interface and lack of an abstract model is that using shell completions (e.g. argcomplete requires importing every applet on every Tab press, which is unusably slow.

    Any solution would have to start off with an argparse API compatibility layer and allow gradual migration. (The new API could either use argparse internally to perform actual argument parsing and help formatting during the migration, or it could present an argparse-compatible interface.)

  • 2. Replace logging with a new, definitely in-house logging system.

    Although logging has served us pretty okay so far (complex applets, especially JTAG-related applets, do already have serious usability problems), there are many issues with it:

    • does not easily support {}-style formats and is the sole remaining reason the codebase uses %-style formats (workarounds are available but unpleasant),
    • does not support multiple TRACE debug levels (requiring hacks such as glasgow -F, instead of straightforward -vvv enabling tracing three interfaces deep),
    • doesn't integrate well with our very specific domain (it would be extremely handy to have nested loggers coupled with the ability to bracket begin/end of a request/response),
    • doesn't support fine-grained elimination of disabled logging calls (ideally through a source-to-source transformation so that hacks like the lazy dump_bin are not necessary in first place),
    • doesn't support structured logging (the structure should be ad-hoc and optimized for writeability/readability of the source, but filtering by e.g. logged addresses would be extremely handy).

    Any solution would include a logging-compatible sink for external library code, as well as to allow gradual migration.

  • 3. Extend the Python import system to allow introducing external (PyPI) library dependencies from applet code.

    Although the current situation where applets directly and unconditionally import libraries and the Glasgow package depends on the union of external dependencies of all in-tree applets has somewhat worked so far, there are many issues with it:

    • is extremely slow to import all packages at all times (the CLI is already too slow for tab-completion or running interactively on RasPi) and is only going to get much slower,
    • is highly error-prone to maintain the import/dependency correspondence manually, especially if version constraints are involved,
    • does not gracefully degrade in case of missing imports (bitarray Eliminate dependency on bitarray #179 is a particularly painful case, being used only by two applets),
    • is inherently incompatible with loading applets directly from filesystem (as individual files) rather than from a setuptools package, as well as graceful handling of imports in even more ad-hoc scenarios such as glasgow script,
    • makes harder to generate option parsers and help text for applets that lack all necessary imports,
    • finally, simply wastes resources (most of the current transitive dependencies are only there because of audio-yamaha-opx, something most people will never use).

    A possible solution is to transform imports of the form from ..pypi.aiohttp import aiohttp or even from ..pypi.aiohttp.ge_3_7 import aiohttp into code that:

    • for packages that are found, simply imports the module from the package,
    • for packages that are not found or do not satisfy a constraint, imports a dummy module that raises an exception if it's being used in any way, records this fact, and ideally propagates it to any modules that import the module with the problematic import,
    • exposes the list of missing imports for any requested module, so that it can be used in the help text, error messages, etc,
    • automatically collects extra_requires for the setuptools package, so that e.g. pip install glasgow[debug-arm-jtag] installs whatever dependencies that applet may or may not have (this becomes even more useful if the new pip resolver starts to track extras statefully rather than statelessly as it is done now).

    It would be extremely cool (and also ensure we never struggle with dependency hell) if the external dependencies of every applet would exist in a sort of "in-process venv", but while this seems possible (barely) with PEP 302 import hooks and creative use of the pip cli, it looks like one of those things that would break in truly horrifying ways that negate whatever advantages they may have when they work.

  • 4. Switch to declarative pin management based on nMigen platforms.

    For historical reasons (namely, Migen platforms being very crude), Glasgow reimplements large chunks of nMigen platform code: connector-based indirection, triple construction, IO buffer insertion. There are many issues with it:

    • prevents us from using nMigen-native pin gearboxes (and thus kneecapping maximum IO frequency of many applets),
    • prevents us from adding multi-applet support because of the rigid argparse-based pin allocation scheme (that would work a lot better if represented as a constraint solving problem),
    • necessitates sprawling XxxBus modules that mostly rearrange the wires attached to the tristate triples that are constructed for every pin,
    • does not have an abstract model that would allow defining FPGA pin constraints (needed at build time) and pull resistor settings (needed at run time) in the same place.

    Unfortunately, Glasgow has additional requirements well beyond what most FPGA boards require (and thus what nmigen-boards can handle):

    • applets may not use a global resource namespace, making direct use of platform.request (with dynamically added resources) non-viable,
    • revC+ has an OE pin that must follow output enable of the corresponding IO pin, even if gearboxes are involved;
    • reuse of subtargets between applets means point of resource definition is completely decoupled from point of resource use, making Decouple pin direction overrides, XDRs, etc from request() amaranth-lang/amaranth#458 a hard requirement;
    • applets use dynamic constraints for pin properties like width range or optionality whereas nMigen only supports static constraints for these properties (i.e. applet resource declarations are higher order compared to nMigen platform resource declarations).

    Solving this will certainly require major nMigen changes (as well as migrating the entire codebase to nMigen in first place).

@whitequark whitequark added software Component: software gateware Component: gateware labels Nov 14, 2020
@whitequark
Copy link
Member Author

  • Extend the Python import system to allow introducing external (PyPI) library dependencies from applet code.

I'm starting to think this might not be necessary (thank gods). The entry point system (available through importlib.metadata) provides almost everything we need.

It seems like there is a reasonably simple solution: don't mess with executable code at all, just add a __requires__ = [...] statement somewhere in the applet module. This statement (where the RHS is strictly a literal) can be easily parsed using
standard functionality. There are a few possible options of what happens next:

  • We might use the entry points functionality closer to the way it's intended: create an "extra" per applet with external dependencies, then add the contents of __requires__ to extra_requires. However, there are a few issues with this. First, metadata can (and will, with editable installs) get outdated. Second, while importlib.metadata will happily give you the dependencies of a package with extras, it won't help you parse it--I think people usually import some distutils gunk which breaks a few months later. Third, while you can easily print a message instructing to pip install <requirements...>, it is much harder to figure out the right command to pip install glasgow[<extras...>] (in fact it's impossible in some cases).
  • Or, we might forget about extras and just manage requirements lists ourselves in a rudimentary way. This is robust with editable installs and so probably preferable, but, annoyingly, still requires parsing dependencies because otherwise there is no way to safely run an applet with dependencies (best case it fails with ImportError, worst case it is silently broken because of one patch revision too low).
  • Maybe both? Try __requires__ first since it is up to date, fall back to extras if that didn't work (maybe a third party chose not to use __requires__).

whitequark added a commit to whitequark/glasgow that referenced this issue Jul 24, 2023
As a concrete example, this (abridged) `pyproject.toml` with
a mandatory dependency on `aiohttp`:

    dependencies = ["aiohttp~=3.8"]

    [project.entry-points."glasgow.applet"]
    audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet"

can now be transformed into the following one with an optional
dependency on `aiohttp`:

  [project.optional-dependencies]
  http = ["aiohttp~=3.8"]

  [project.entry-points."glasgow.applet"]
  audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet [http]"

If `aiohttp` is not installed or its version is unsatisfying, the CLI
shows a placeholder instead of the applet whose `--help` text explains
how to install the missing packages.

See GlasgowEmbedded#234.
whitequark added a commit to whitequark/glasgow that referenced this issue Jul 24, 2023
As a concrete example, this (abridged) `pyproject.toml` with
a mandatory dependency on `aiohttp`:

    dependencies = ["aiohttp~=3.8"]

    [project.entry-points."glasgow.applet"]
    audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet"

can now be transformed into the following one with an optional
dependency on `aiohttp`:

  [project.optional-dependencies]
  http = ["aiohttp~=3.8"]

  [project.entry-points."glasgow.applet"]
  audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet [http]"

If `aiohttp` is not installed or its version is unsatisfying, the CLI
shows a placeholder instead of the applet whose `--help` text explains
how to install the missing packages.

See GlasgowEmbedded#234.
whitequark added a commit to whitequark/glasgow that referenced this issue Jul 24, 2023
As a concrete example, this (abridged) `pyproject.toml` with
a mandatory dependency on `aiohttp`:

    dependencies = ["aiohttp~=3.8"]

    [project.entry-points."glasgow.applet"]
    audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet"

can now be transformed into the following one with an optional
dependency on `aiohttp`:

  [project.optional-dependencies]
  http = ["aiohttp~=3.8"]

  [project.entry-points."glasgow.applet"]
  audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet [http]"

If `aiohttp` is not installed or its version is unsatisfying, the CLI
shows a placeholder instead of the applet whose `--help` text explains
how to install the missing packages.

See GlasgowEmbedded#234.
whitequark added a commit to whitequark/glasgow that referenced this issue Jul 31, 2023
As a concrete example, this (abridged) `pyproject.toml` with
a mandatory dependency on `aiohttp`:

    dependencies = ["aiohttp~=3.8"]

    [project.entry-points."glasgow.applet"]
    audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet"

can now be transformed into the following one with an optional
dependency on `aiohttp`:

  [project.optional-dependencies]
  http = ["aiohttp~=3.8"]

  [project.entry-points."glasgow.applet"]
  audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet [http]"

If `aiohttp` is not installed or its version is unsatisfying, the CLI
shows a placeholder instead of the applet whose `--help` text explains
how to install the missing packages.

See GlasgowEmbedded#234.
github-merge-queue bot pushed a commit that referenced this issue Aug 1, 2023
As a concrete example, this (abridged) `pyproject.toml` with
a mandatory dependency on `aiohttp`:

    dependencies = ["aiohttp~=3.8"]

    [project.entry-points."glasgow.applet"]
    audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet"

can now be transformed into the following one with an optional
dependency on `aiohttp`:

  [project.optional-dependencies]
  http = ["aiohttp~=3.8"]

  [project.entry-points."glasgow.applet"]
  audio-yamaha-opx = "glasgow.applet.audio.yamaha_opx:AudioYamahaOPxApplet [http]"

If `aiohttp` is not installed or its version is unsatisfying, the CLI
shows a placeholder instead of the applet whose `--help` text explains
how to install the missing packages.

See #234.
@whitequark
Copy link
Member Author

  1. Extend the Python import system to allow introducing external (PyPI) library dependencies from applet code.

The third goal is now complete! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gateware Component: gateware software Component: software
Projects
None yet
Development

No branches or pull requests

1 participant