Skip to content

Fix pgrep length limit - #31

Open
niveK77pur wants to merge 1 commit into
Slimbook-Team:mainfrom
niveK77pur:main
Open

Fix pgrep length limit#31
niveK77pur wants to merge 1 commit into
Slimbook-Team:mainfrom
niveK77pur:main

Conversation

@niveK77pur

Copy link
Copy Markdown

This fix is related to Slimbook-Team/slimbookbattery@5ec4ab9.

This patch makes the program run in Debian and other systems. In them, pgrep has a limit of 15 characters, so the "slimbookbatterypreferences" string is too long for it. This patch uses the -f parameter to fix this.


I was working on a rather elaborate fix for this, since I did not know whether pgrep's -f option is available across all versions and systems. However, as I was getting ready to make a fork for a PR, I spotted the linked commit on the slimbookbattery repository to address the issue (I am having the exact same issue with that application as well). I also spot pgrep -f in other places in this repository, so I suppose the flag was simply forgotten here.

I hope I am not overlooking something, which makes the exclusion of -f intentional. Reason for this fix, is that I cannot launch the slimbookamdcontroller myself, even if it is not running (it prints to the terminal that the application is already running) — only way to launch it is to allow it at startup. After some digging, I found the current pgrep command to spit out an error, which triggers the following if conditions wrongly.

P.S. This is my first every code contribution to a project!

This fix is related to Slimbook-Team/slimbookbattery@5ec4ab9.

> This patch makes the program run in Debian and other systems. In them, pgrep has a limit of 15 characters, so the "slimbookbatterypreferences" string is too long for it. This patch uses the -f parameter to fix this.
@Lt-Henry

Lt-Henry commented Sep 7, 2023

Copy link
Copy Markdown
Contributor

Hi, thanks for the patch. I am currently looking into it but adding -f option makes application complain about already loaded when It is not. I think (not the author of this code) this initialization is broken, no matter if -f or not. Later in the initialization sequence, there is another call to pgrep -f which it seems to work properly (using Popen instead of getoutput)

@jcarolinares

jcarolinares commented Aug 14, 2024

Copy link
Copy Markdown

I confirm what @Lt-Henry is saying. With a freshly upgrade Ubuntu 24 from version Ubuntu 22 the -f command is not enough

With Ubuntu 22 I've fixed some time days ago following: #30 and installing with PIP

pip install pyamdgpuinfo

In this case is not working and due to the changes of ubuntu 24 it is more confusing:

carolinares@jcarolinares-TITAN:~$ pip install pyamdgpuinfo
error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try apt install
    python3-xyz, where xyz is the package you are trying to
    install.
    
    If you wish to install a non-Debian-packaged Python package,
    create a virtual environment using python3 -m venv path/to/venv.
    Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
    sure you have python3-full installed.
    
    If you wish to install a non-Debian packaged Python application,
    it may be easiest to use pipx install xyz, which will manage a
    virtual environment for you. Make sure you have pipx installed.
    
    See /usr/share/doc/python3.12/README.venv for more information.

note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.

hopefully this can bring more clues, thank you both!

EDIT: I see the AMD Controller disappeared from Ubuntu 24.04 not being possible of installing it anymore. I get it if it is not working I just wonder if you guys have plans to it soon or how can I control the Ryzen TPU via commands in the meantime, thanks!

@Lt-Henry

Copy link
Copy Markdown
Contributor

I confirm what @Lt-Henry is saying. With a freshly upgrade Ubuntu 24 from version Ubuntu 22 the -f command is not enough

With Ubuntu 22 I've fixed some time days ago following: #30 and installing with PIP

pip install pyamdgpuinfo

In this case is not working and due to the changes of ubuntu 24 it is more confusing:

carolinares@jcarolinares-TITAN:~$ pip install pyamdgpuinfo
error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try apt install
    python3-xyz, where xyz is the package you are trying to
    install.
    
    If you wish to install a non-Debian-packaged Python package,
    create a virtual environment using python3 -m venv path/to/venv.
    Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
    sure you have python3-full installed.
    
    If you wish to install a non-Debian packaged Python application,
    it may be easiest to use pipx install xyz, which will manage a
    virtual environment for you. Make sure you have pipx installed.
    
    See /usr/share/doc/python3.12/README.venv for more information.

note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.

hopefully this can bring more clues, thank you both!

EDIT: I see the AMD Controller disappeared from Ubuntu 24.04 not being possible of installing it anymore. I get it if it is not working I just wonder if you guys have plans to it soon or how can I control the Ryzen TPU via commands in the meantime, thanks!

We have refactored both amd and intel controller, cleaning up a bit the code, fixing some bugs and improving package quality a bit.

From 24.04, SlimbookOS no longer uses launchpad but its own repository. We are however, keeping launchpad for those who already configured it and are more comfortable with a vanilla Ubuntu. Give me a couple of days and I will move amdcontroller to launchpad too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants