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

Support for OSTREE-based distributions #181

Merged
merged 7 commits into from
Aug 14, 2024

Conversation

PilotGuy772
Copy link
Contributor

@PilotGuy772 PilotGuy772 commented Aug 11, 2024

Fixes #180
Fixes #114
Fixes #49

EnvyControl is configured right now to put udev rules in /lib/udev. The problem with this is that /lib/udev is distinctly intended for udev rules from the distribution. It is NOT meant to be manipulated so heavily. Instead, system admins ought to use /etc/udev to put custom rules.

This ordinarily isn't a problem for EnvyControl because most distros don't care if you modify /lib/, but immutable distros like Fedora Silverblue do care. /lib/ is not writable, but /etc/ is. There's a reason for that.

This PR changes the udev directory to /etc/udev/, but everything still works as expected with this change. I also added logic to check if the distribution uses ostree by checking for /ostree and /sysroot/ostree, which are the two places where you would usually find the ostree directory. My amateur testing on Bluefin has shown that switching between integrated and hybrid modes works perfectly.

envycontrol.py Show resolved Hide resolved
envycontrol.py Show resolved Hide resolved
@PilotGuy772
Copy link
Contributor Author

@bayasdev I added logic to cleanup() in my latest commit to address your concerns. I just have statements to check for EnvyControl's udev rules in /lib/udev/rules.d and remove them if they exist.

envycontrol.py Outdated
# attempt to delete rules from legacy directory
legacy_udev = '/lib/udev/rules.d/50-remove-nvidia.rules'
legacy_udev_pm = '/lib/udev/rules.d/80-nvidia-pm.rules'
if os.path.exists(legacy_udev):
Copy link
Owner

Choose a reason for hiding this comment

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

@PilotGuy772 I think we could do something like this instead

    # define list of files to remove
    to_remove = [
        BLACKLIST_PATH,
        UDEV_INTEGRATED_PATH,
        UDEV_PM_PATH,
        XORG_PATH,
        EXTRA_XORG_PATH,
        MODESET_PATH,
        LIGHTDM_SCRIPT_PATH,
        LIGHTDM_CONFIG_PATH,
        # legacy paths
        '/etc/X11/xorg.conf.d/90-nvidia.conf',
        '/lib/udev/rules.d/50-remove-nvidia.rules',
        '/lib/udev/rules.d/80-nvidia-pm.rules'
    ]

    # remove each file in the list
    for file_path in to_remove:
        try:
            if os.path.exists(lfile_path):
                os.remove(file_path)
                logging.info(f"Removed file {file_path}")
        except OSError as e:
            # only warn if file exists (code 2)
            if e.errno != 2:
                logging.error(f"Failed to remove file '{file_path}': {e}")

envycontrol.py Outdated
# I'm keeping --force, but --regenerate-all is not necessary
# because rpm-ostree already passes the kernel version.

# this takes a LLOOOOOONG time to run, so let's warn the user first.
Copy link
Owner

Choose a reason for hiding this comment

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

could you please remove the redundant comments?

envycontrol.py Outdated
@@ -600,7 +632,7 @@ class CachedConfig:

def __init__(self, app_args) -> None:
self.app_args = app_args
self.current_mode = get_current_mode()
self.current_mode = get_nvidia_gpu_pci_bus()
Copy link
Owner

Choose a reason for hiding this comment

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

why was this changed?

@bayasdev
Copy link
Owner

@PilotGuy772 please take a look at the comments I left, and thank you for adding OSTree support to EnvyControl as it was a frequently requested feature 🙌🏼

@PilotGuy772
Copy link
Contributor Author

@bayasdev All done (I think)

Copy link
Owner

@bayasdev bayasdev left a comment

Choose a reason for hiding this comment

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

LGTM

@bayasdev bayasdev merged commit 7a52eca into bayasdev:main Aug 14, 2024
@bayasdev
Copy link
Owner

Thanks @PilotGuy772

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants