infra: Remove unnecessary deps from install deps #5844
Conversation
|
/kickstart-test --waive infra only |
There was a problem hiding this comment.
This should rather be properly implemented through an --disable-bootc argument in configure.ac
Then the spec file can conditionally build anaconda without bootc dependencies.
I am not expert in build tools, so we can pull in someone else for opinion, but I am pretty sure this solution here suboptimal to the suggested one.
|
I don't think this have to be Also do we want to complicate our code because of this script. Seems to me too much effort for a little gain. |
|
Also when I'm thinking about it, it won't help here. Having support in configure.ac won't be usable by the script because the script is not building the RPM but only reading spec file. It don't have any information from the configure step. |
| @@ -42,7 +42,9 @@ sed 's/@PACKAGE_VERSION@/0/; s/@PACKAGE_RELEASE@/0/; s/%{__python3}/python3/' ./ | |||
| build_deps=$(rpmspec -q --buildrequires $TEMP | sed 's/>=.*$//') | |||
| # add also runtime dependencies for the local development | |||
| # remove anaconda packages and also '(glibc-langpack-en or glibc-all-langpacks)' which will fail otherwise | |||
| requires_deps=$(rpmspec -q --requires $TEMP | grep -v -E "(anaconda-|-widgets| or )" | sed 's/>=.*$//') | |||
| # remove podman, ostree (and skopeo as ostree dependency) because these are not required for development of Anaconda | |||
| # and are breaking easy use of host commands from the container (distrobox-host-exec) | |||
There was a problem hiding this comment.
You can do something like:
`rpmspec --define "build_bootc 0 ..."
There was a problem hiding this comment.
And in the spec file:
diff --git anaconda.spec.in anaconda.spec.in
index 709ab7fcc2..0602d95d42 100644
--- anaconda.spec.in
+++ anaconda.spec.in
@@ -46,6 +46,7 @@ Source0: https://github.com/rhinstaller/%{name}/releases/download/%{name}-%{vers
%define subscriptionmanagerver 1.26
%define utillinuxver 2.15.1
%define rpmostreever 2023.2
+%define build_bootc 1
BuildRequires: libtool
BuildRequires: gettext-devel >= %{gettextver}
@@ -273,7 +274,9 @@ Requires: skopeo
# External tooling for managing NVMe-FC devices in the installation environment
Requires: nvme-cli
# Needed for bootc
+%if %{build_bootc}
Requires: podman
+%endifThere was a problem hiding this comment.
This is great. I wasn't aware that rpmspec supports this. Sounds good.
|
What about external contributors ? I think some of them might be using this script prepare their environment for Anaconda development ? |
You mean installing the deps on their machine directly and not container I guess? Installing the RPM dependencies just helps code editor programs to not complain about missing imports. Nothing more than that. Not sure what you see as problematic here. |
|
@KKoukiou asked for opinions here. My gut feeling about the current PR change is "hackish, but bearable". The goal is 100% legit -- it doesn't make sense to pull in podman & friends onto developer laptops or toolboxes. Whether you do this with rpm conditionals or sed doesn't make too much practical difference. FTR, for my own toolboxes I use flatpak-spawn wrappers to call things like |
Thank you Martin. |
Distrobox has a great solution for this. It works in a way that you will symlink the |
One of the biggest use cases for the install_dependencies.sh script is to get dependencies to the container for development easily. However, container management services are not usable from the container directly and are not helpful for development (just runtime dependency). Having these in the container could block easy way to call binaries from inside of the container in similar cases like `distrobox-host-exec` which enables you to call container commands (allows our make commands) from inside of the distrobox container. Others are just wrong to be installed on developer machine for example gnome-kiosk.
jkonecny12
force-pushed
the
master-remove-podman-from-install-dependencies
branch
from
3ec3319
to
3d5a065
Compare
jkonecny12
changed the title
|
OK, I changed this to a list of dependencies which shouldn't be on the developer machine. This will make size of the deps almost halved. |
|
As before, this is not requirement to merge. I would honestly like to hear what do you think @KKoukiou |
|
Closed in favor of #5854 |
One of the biggest use cases for the install_dependencies.sh script is to get dependencies to the container for development easily. However, container management services are not usable from the container directly and are not helpful for development (just runtime dependency).
These packages are:
Having these in the container is blocking easy way to call these from inside of the container in similar cases like
distrobox-host-execwhich enables you to call container commands (allows our make commands) from inside of the distrobox container.