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

pineflash doesn't find binaries (pkexec) because it overrides the PATH variable #89

Open
nydragon opened this issue Jan 6, 2025 · 7 comments

Comments

@nydragon
Copy link

nydragon commented Jan 6, 2025

Hardcoding the PATH makes Command unable to find the pkexec binary on NixOS:

.env("PATH", "/usr/local/sbin:/usr/local/bin:/usr/bin:/bin:linux")

Removing this line (82605e4) fixes the issue, and as the hardcoded values are already contained in PATH by default, removing it should not break anything on other distros.

Can submit a PR if wished.

@acuteaangle
Copy link
Contributor

I’m assuming you’re using the appimage?

Interestingly, the dfu-util version of this function had this this commented out in e263b0fc8 ‘appimage improvements’.

I wasn’t able to find the original reason that line was included.

@nydragon
Copy link
Author

nydragon commented Jan 7, 2025

I am building the program from source as i wanted to make it run natively on my machine

@acuteaangle
Copy link
Contributor

Were you able to run PineFlash without patching the binary with patchelf? In my testing for the Nixpkgs PR, patchelf was necessary to add libxkbcommon and libGL to rpath.

@nydragon
Copy link
Author

nydragon commented Jan 7, 2025

I added the library paths to LD_LIBRARY_PATH using wrapProgram (link) and patched out the aforementioned line, but I'm not sure what is the better practice.

Also i think you don't need the patch you added to your PR, works fine without

@acuteaangle
Copy link
Contributor

The patch was necessary in my testing, as otherwise it would find the unwrapped pkexec and fail complaining that it wasn’t SUID. I assume wrapProgram is related to the differing behavior.

@nydragon
Copy link
Author

nydragon commented Jan 8, 2025

Ah alright then, but what about the issue ? Would removing the line break AppImage compat ?

@acuteaangle
Copy link
Contributor

The appimage compat commit was making essentially the same change as proposed here but for the dfu-util path.
I’m not super familiar with the internals of appimages, but it seems like this was also causing some sort of problem there.

I don’t have a non-NixOS system currently to test if this would break anything elsewhere.
I’d really like it if PineFlash had an end-to-end test.

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

No branches or pull requests

2 participants