search

masukomi / private_comments

a tool for managing private comments on, but not in, your files
48 stars 2 forks source link

The build scripts should not require `sudo` to install #22

Open filmil opened 1 year ago

filmil commented 1 year ago

Following build instructions from the README.md, trying to build private_comments for 64-bit linux, I noticed that the build scripts want to install into paths that are typically not writable for a regular user.

It would be nice for that requirement to be removed, it does not seem necessary.

I could sudo run the install scripts, but that seems like an overreach.

masukomi commented 12 months ago

Sorry i didn't see this earlier. Can you be more specific?

If you're talking about install_chicken_eggs.sh, that just uses chicken-install which chicken scheme's package manager. I don't have any control over where that installs things.

with regards to homebrew install instructions that's another package manager I don't have control over the installation destination of.

Was it one of those or something else?

filmil commented 12 months ago

It's the chicken-install bit that's problematic. (And I'm installing on Linux, so homebrew does not apply, I had to turn that bit off.)

In 2023, it's not OK to require root access to the machine just to install a compiler and/or some source code. I'd recommend finding another way to bring the dependencies in.

masukomi commented 11 months ago

In 2023, it's not OK to require root access to the machine just to install a compiler and/or some source code. I'd recommend finding another way to bring the dependencies in.

Strong agree. I hate that, but again I'm not the one requiring it. chicken-install is the package manager for Chicken Scheme and there aren't any alternatives. My best guess is that when you installed chicken scheme you did so with sudo and thus future chicken related things are requiring it? When I installed chicken scheme on my machine I didn't use sudo, and chicken-install never asks for root perms to install things. This sounds like either user error or someone screwing up in how they added Chicken Scheme to your OS's package manager.

This just isn't something i really have control over without creating a new package manager + dependency checker for Chicken Scheme. ;)

filmil commented 11 months ago

Noted. I would consider checking if there is a way to have a private installation of the runtime for a user, regardless of what the system does. May well be that it's not possible. But I'm not familiar with Chicken Scheme to know for sure. And I suppose that building static binaries would remove any such concerns.

I have in the past solved such issues elsewhere by making a dockerized build environment used to produce self-contained binaries. Not saying that you should endeavor any such thing, but just noting it as an option. If this is something you'd entertain I might be able to contribute a fix for https://github.com/masukomi/private_comments/issues/21.

Deployment details aside, I think the private comments idea is brilliant, and I wonder why there are so few projects that offer this.

Sadly this project is not directly usable for me in its current form due to security concerns (https://github.com/masukomi/private_comments/issues/23) but I may have some avenues to fix that, in which case I might try to fix, and contribute back anything I can.

filmil commented 11 months ago

Also see https://github.com/masukomi/private_comments/issues/25

masukomi commented 11 months ago

I'm intrigued by the LSP idea. That approach hadn't occurred to me.

I'd be happy to accept a PR for a docker build thing. That sounds like something that could be done without much effort by someone who's more familiar with Docker than I.

I don't think the LSP idea should be attempted until this bug is fixed. It's not that it's a blocker but that it's a mildly frustrating / confusing issue that should probably be nailed down before this gets many more folks using it.

filmil commented 11 months ago

I'm intrigued by the LSP idea. That approach hadn't occurred to me.

I suppose it occurred to me because I'm lazy and don't want to develop frontend. :)

I'd be happy to accept a PR for a docker build thing. That sounds like something that could be done without much effort by someone who's more familiar with Docker than I.

https://github.com/masukomi/private_comments/pull/26

I did take some liberties there that I don't know whether you'd approve of. But the benefits outweigh the concerns IMHO, since this way (and with a bit more changes, but not too many) you can build and release the binaries right here on GitHub. I'm testing an action that will build a new binary when you tag a new release.

I don't think the LSP idea should be attempted until this bug is fixed. It's not that it's a blocker but that it's a mildly frustrating / confusing issue that should probably be nailed down before this gets many more folks using it.

Up to you.