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

dracut: depend on net-lib not ifcfg #6125

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

AdamWill
Copy link
Contributor

ifcfg is gone in dracut-ng 204. Looking at the module and its history, I'm pretty sure we don't actually use anything from the ifcfg module any more. anaconda-ifcfg.sh only uses save_netinfo, which is in net-lib. So let's turn the ifcfg dep into a net-lib dep instead.

If I missed anything and we really are still relying on the ifcfg module then we need to fix that, since it's gone now.

@github-actions github-actions bot added the f42 Fedora 42 label Jan 31, 2025
Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good as far as I can tell, but @rvykydal should ideally take a look as well. :)

@rvykydal
Copy link
Contributor

rvykydal commented Feb 6, 2025

/kickstart-test --testtype smoke

Copy link
Contributor

@rvykydal rvykydal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@AdamWill
Copy link
Contributor Author

AdamWill commented Feb 7, 2025

ping?

@rvykydal
Copy link
Contributor

/kickstart-test --testtype smoke

@M4rtinK M4rtinK mentioned this pull request Feb 10, 2025
2 tasks
@@ -8,7 +8,7 @@ check() {
}

depends() {
echo livenet nfs img-lib convertfs ifcfg
echo livenet nfs img-lib convertfs net-lib
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not mean to slow down progress here, but this could be further simplified to
livenet nfs convertfs .

img-lib and net-lib are already transitive dependencies of livenet.

Keeping the list of dependencies to the "top level" dracut modules, could avoid future compat issues like this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally, I subscribe to the opinion that direct dependencies should always be explicit. Assuming anaconda directly uses img-lib, it is correct for anaconda to have a direct dep on it. Relying on transitive dependencies is fundamentally unsafe - sure, livenet uses img-lib now, but what if livenet is later changed upstream to not use img-lib? anaconda would suddenly break.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AdamWill agreed. It seems img-lib is directly used, but net-lib is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as noted in the commit message, the anaconda module uses save_netinfo, which is from net-lib.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

In that case I would have also sourced net-lib from anaconda-ifcfg.sh (which is why I missed it).

Also noticed that anaconda-lib.sh sources url-lib which might warrant to have url-lib listed as a direct dependency.

These changes might be out of scope for this kind of unblocking PR..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i think those are out of scope for this PR.

ifcfg is gone in dracut-ng 204. Looking at the module and its
history, I'm pretty sure we don't actually use anything from the
ifcfg module any more. anaconda-ifcfg.sh only uses save_netinfo,
which is in net-lib. So let's turn the ifcfg dep into a net-lib
dep instead.

If I missed anything and we really are still relying on the ifcfg
module then we need to fix that, since it's gone now.

Resolves: rhbz#2343125
@rvykydal
Copy link
Contributor

/kickstart-test --testtype smoke

@rvykydal rvykydal added the f43 label Feb 12, 2025
@rvykydal rvykydal merged commit 1768e16 into rhinstaller:main Feb 12, 2025
19 checks passed
@KKoukiou KKoukiou removed the f42 Fedora 42 label Feb 13, 2025
achilleas-k added a commit to achilleas-k/images that referenced this pull request Feb 19, 2025
In RHEL 10 and Fedora 42, the ifcfg module was replaced by net-lib.
Anaconda changes:
- rhinstaller/anaconda#6125
- rhinstaller/anaconda#6155

Remove ifcfg from common anaconda dracut stage modules and add either
ifcfg or net-lib to each installer based on distro version.
achilleas-k added a commit to achilleas-k/images that referenced this pull request Feb 19, 2025
In RHEL 10 and Fedora 42, the ifcfg module was replaced by net-lib.
Anaconda changes:
- rhinstaller/anaconda#6125
- rhinstaller/anaconda#6155

Remove ifcfg from common anaconda dracut stage modules and add either
ifcfg or net-lib to each installer based on distro version.
achilleas-k added a commit to achilleas-k/images that referenced this pull request Feb 19, 2025
In RHEL 10 and Fedora 42, the ifcfg module was replaced by net-lib.
Anaconda changes:
- rhinstaller/anaconda#6125
- rhinstaller/anaconda#6155

Remove ifcfg from common anaconda dracut stage modules and add either
ifcfg or net-lib to each installer based on distro version.
github-merge-queue bot pushed a commit to osbuild/images that referenced this pull request Feb 20, 2025
In RHEL 10 and Fedora 42, the ifcfg module was replaced by net-lib.
Anaconda changes:
- rhinstaller/anaconda#6125
- rhinstaller/anaconda#6155

Remove ifcfg from common anaconda dracut stage modules and add either
ifcfg or net-lib to each installer based on distro version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

6 participants