Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The Makefile refactoring for multi-distro support introduces a critical duplicate target error that breaks Fedora builds. Several Ubuntu package dependencies do not exist in standard repositories, which will cause installation to fail. A Fedora dependency also uses a file path instead of a package name.
happz
force-pushed
the
makefile-ubuntu-deps
branch
from
dcc2ceb to
02e6f5e
Compare
| _DEPS_CORE_FEDORA := \ | ||
| hatch \ | ||
| uv \ | ||
| python3-devel \ | ||
| python3-hatch-vcs \ | ||
| rpm-build \ | ||
| clang \ | ||
| beakerlib | ||
|
|
||
| _DEPS_DEVELOP_FEDORA := \ | ||
| expect \ | ||
| gcc \ | ||
| git \ | ||
| python3-nitrate \ | ||
| libvirt-devel \ | ||
| krb5-devel \ | ||
| libpq-devel \ | ||
| python3-devel \ | ||
| jq \ | ||
| podman \ | ||
| buildah \ | ||
| /usr/bin/python3.9 \ | ||
| pre-commit | ||
|
|
||
| # Note: interestingly, `hatch` is not shipped by Ubuntu, but `hatch` plugins are. | ||
| # Manual `hatch` installation probably does not the packaged plugin, leaving it | ||
| # in for now until we get better coverage. | ||
| _DEPS_CORE_UBUNTU := \ | ||
| python3-dev \ | ||
| python3-hatch-vcs \ | ||
| clang | ||
|
|
||
| _DEPS_DEVELOP_UBUNTU := \ | ||
| expect \ | ||
| gcc \ | ||
| git \ | ||
| libvirt-dev \ | ||
| libkrb5-dev \ | ||
| libpq-dev \ | ||
| python3-dev \ | ||
| jq \ | ||
| podman \ | ||
| buildah \ | ||
| python3.9 \ | ||
| pre-commit |
There was a problem hiding this comment.
Can reshuffle so that cores are together with core and develop are together with develop?
There was a problem hiding this comment.
I had it that way, then I switched to per-distro grouping. I guess both have their pros and cons, I don't have a favorite.
There was a problem hiding this comment.
Hmm, yeah. In one case we have all of the dependencies grouped together so it is easier for the maintainer to make sure something is not missed. In the other case it might be more useful for the passer-by checking the content
A small refactoring of related part of the Makefile, to separate distro-specific targets and lists.
happz
force-pushed
the
makefile-ubuntu-deps
branch
from
6bea484 to
58e6cec
Compare
2 participants
A small refactoring of related part of the Makefile, to separate
distro-specific targets and lists.
Pull Request Checklist