Switch Review Board to a pyproject.toml-based packaging setup.

Review Request #14291 — Created Jan. 14, 2025 and updated

Information

Review Board
release-7.1.x

Reviewers

With legacy packaging centered around setup.py now on the way out, we
need to move to modern packaging using pyproject.toml and build
backends. The challenge is that Review Board's build setup is a bit more
involved than your average Python package, with static media building
and gettext i18n file generation being part of the process, and a fair
amount of setup required to get there.

This change introduces a build backend based on the work we did for
Djblets's backend, but with a few extra capabilities to ease
development and support the setup requirements, and doing so in pip's
isolated build environment.

The build backend handles a few custom steps:

  1. Dependency management
  2. Static media setup and building
  3. i18n building

Dependency management is a bit special, supporting testing against local
development packages.

When building a package, we pull dependencies from
reviewboard/dependencies.py and serialize them to
package-requirements.txt. This file is then consumed by the underlying
setuptools backend, turning into the list of dependencies for a package.

When installing in editable mode, it goes through the list of
dependencies and looks for any that has a matching symlink in
.local-packages/.

If one is found, then that path is used for the dependency, allowing us
to build and test against a local package. We do this automatically for
Djblets when invoked by ./contrib/internal/prepare-dev.py.

The result is then used as the list of dependencies for the temporary
build environment, and for the .npm-workspaces symlinks.

Static media setup uses .npm-workspaces as before, but it's now built
automatically when we pip install -e or build a package.

Note that any symlinks in .npm-workspaces will get replaced with links
to temporary locations when building a package, but this is all the more
reason to only build a package in a clean clone (beanbag-tools does
this automatically).

Along with these changes, MANIFEST.in has been updated with several
files and exclusions needed to generate a suitable source distribution.
We previously had files included that should not have been there and
other files missing. The list is now more comprehensive.

Given the straightjacket that is the Python build environment, some of
these specialized steps are very non-standard. Though this should not
break, it does require further testing. We'll aim to ship this as part
of Review Board 7.1.

Tested setting up a new development tree using prepare-dev.py with a
local Djblets tree (and a public build, but we can't build static media
with that, lacking shipped build tools in the Wheel). Verified that I
could run Review Board and all static media loaded, and that the links
in .local-packages and .npm-workspaces were correct.

Tested manually setting up editable mode without prepare-dev.py.

Tested building wheels and source distributions (together and
independently), and compared to the setup.py-based builds. This was
tested in both a clean Git clone and my development tree. I checked
the files in the builds for expected content.

Summary ID
Switch Review Board to a pyproject.toml-based packaging setup.
With legacy packaging centered around `setup.py` now on the way out, we need to move to modern packaging using `pyproject.toml` and build backends. The challenge is that Review Board's build setup is a bit more involved than your average Python package, with static media building and gettext i18n file generation being part of the process, and a fair amount of setup required to get there. This change introduces a build backend based on the work we did for Djblets's backend, but with a few extra capabilities to ease development and support the setup requirements, and doing so in pip's isolated build environment. The build backend handles a few custom steps: 1. Dependency management 2. Static media setup and building 3. i18n building Dependency management is a bit special, supporting testing against local development packages. When building a package, we pull dependencies from `reviewboard/dependencies.py` and serialize them to `package-requirements.txt`. This file is then consumed by the underlying setuptools backend, turning into the list of dependencies for a package. When installing in editable mode, it goes through the list of dependencies and looks for any that has a matching symlink in `.local-packages/`. If one is found, then that path is used for the dependency, allowing us to build and test against a local package. We do this automatically for Djblets when invoked by `./contrib/internal/prepare-dev.py`. The result is then used as the list of dependencies for the temporary build environment, and for the `.npm-workspaces` symlinks. Static media setup uses `.npm-workspaces` as before, but it's now built automatically when we `pip install -e` or build a package. Note that any symlinks in `.npm-workspaces` will get replaced with links to temporary locations when building a package, but this is all the more reason to only build a package in a clean clone (`beanbag-tools` does this automatically). Along with these changes, `MANIFEST.in` has been updated with several files and exclusions needed to generate a suitable source distribution. We previously had files included that should not have been there and other files missing. The list is now more comprehensive. Given the straightjacket that is the Python build environment, some of these specialized steps are very non-standard. Though this should not break, it does require further testing. We'll aim to ship this as part of Review Board 7.1.
a30808bb0969f4c9bb613756f8d979d511e5103f
Description From Last Updated

In our other repos we have a Makefile which is nice when I don't want to type out the whole …

maubinmaubin

I created a djblets symlink (named Djblets) in .local-packages. Then I ran prepare-dev.py and encountered this error. (reviewboard-7.1-plus) maubin@Mich-Thinkpad:~/src/trees/reviewboard/release-7.1.x$ ./contrib/inte …

maubinmaubin

Tested running prepare-dev.py with no .local-packages dir, and it completed successfully. But when going to the website I ran into: …

maubinmaubin

'typing.Literal' imported but unused Column: 1 Error code: F401

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

maubin
  1. I created a new virtualenv and tested out setting up RB in a few different ways, as detailed in the last 3 comments below.

  2. Show all issues

    In our other repos we have a Makefile which is nice when I don't want to type out the whole pip install --no-build-isolation -e .. Can we add one here too?

  3. Show all issues

    I created a djblets symlink (named Djblets) in .local-packages. Then I ran prepare-dev.py and encountered this error.

    (reviewboard-7.1-plus) maubin@Mich-Thinkpad:~/src/trees/reviewboard/release-7.1.x$ ./contrib/inte
    rnal/prepare-dev.py
    Traceback (most recent call last):
      File "/home/maubin/src/trees/reviewboard/release-7.1.x/./contrib/internal/prepare-dev.py", line 562, in <module>
        main()
      File "/home/maubin/src/trees/reviewboard/release-7.1.x/./contrib/internal/prepare-dev.py", line 487, in main
        setup_local_packages()
      File "/home/maubin/src/trees/reviewboard/release-7.1.x/./contrib/internal/prepare-dev.py", line 204, in setup_local_packages
        os.symlink(tree_path,
    FileExistsError: [Errno 17] File exists: '/home/maubin/src/trees/djblets/release-5.x' -> '.local-packages/Djblets'
    

    But prepare-dev.py should still work even if things exist in .local-packages, right?

  4. Show all issues

    Tested running prepare-dev.py with no .local-packages dir, and it completed successfully. But when going to the website I ran into:

    Your Review Board installation does not have a /home/maubin/src/trees/reviewboard/release-7.1.x/reviewboard/htdocs/media/uploaded/images directory.
    

    Not sure if this is expected, but I've never had to do this in the past. So I followed the instructions for creating the media/uploaded/images dir and restarted the server. Then everything worked as normal.

    Except I did see this JS error on an empty review request page, which is probably not related to this change but I'll put it here anyways

    Error compiling JavaScript package "rbintegrations.extension.RBIntegrationsExtension-fields"
    Command:
    
    /home/maubin/src/trees/reviewboard/release-7.1.x/node_modules/.bin/rollup --sourcemap -i /home/maubin/envs/reviewboard-7.1-plus/lib/python3.9/site-packages/rbintegrations/static/js/fields/index.ts -o /home/maubin/src/trees/reviewboard/release-7.1.x/reviewboard/htdocs/static/ext/rbintegrations.extension.RBIntegrationsExtension/js/fields/index.js
    Errors:
    
    /home/maubin/envs/reviewboard-7.1-plus/lib/python3.9/site-packages/rbintegrations/static/js/fields/index.ts ? reviewboard/htdocs/static/ext/rbintegrations.extension.RBIntegrationsExtension/js/fields/index.js...
    [!] RollupError: Could not resolve "./asanaFieldView" from "../../../../envs/reviewboard-7.1-plus/lib/python3.9/site-packages/rbintegrations/static/js/fields/index.ts"
    ../../../../envs/reviewboard-7.1-plus/lib/python3.9/site-packages/rbintegrations/static/js/fields/index.ts
        at error (/home/maubin/src/trees/reviewboard/release-7.1.x/node_modules/rollup/dist/shared/rollup.js:210:30)
        at ModuleLoader.handleInvalidResolvedId (/home/maubin/src/trees/reviewboard/release-7.1.x/node_modules/rollup/dist/shared/rollup.js:23653:24)
        at /home/maubin/src/trees/reviewboard/release-7.1.x/node_modules/rollup/dist/shared/rollup.js:23615:26
    
  5. Tested running python3.x -m pip install -e . for python3.8-3.11. Ran the dev server with python3.10, it worked normally.

  6. 
      
misery
  1. 
      
  2. pyproject.toml (Diff revision 1)
     
     
     
     
     
     
     

    Do you know https://github.com/pypa/hatch ? :-) Don't know if it fits for you.

    1. Yep, I've evaluated hatch carefully. It's good! But it doesn't outright solve any of the problems we need solved, and we already have plenty of infrastructure hosted on setuptools. We needed our own build backend in order to do the packaging steps needed, and while Hatch has some plugin support, it ended up being more work and more things to work around to get the same results. But our use case is pretty special. For other projects, I'd recommend hatch.

  3.