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.

Sign up for GitHub

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

Jump to bottom

The build scripts should not require sudo to install #22

Open
filmil opened this issue Sep 18, 2023 · 7 comments
Open

The build scripts should not require sudo to install #22

filmil opened this issue Sep 18, 2023 · 7 comments

Comments

@filmil
Copy link
Contributor

filmil commented Sep 18, 2023

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
Copy link
Owner

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
Copy link
Contributor Author

filmil commented Oct 26, 2023

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
Copy link
Owner

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
Copy link
Contributor Author

filmil commented Nov 15, 2023

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 #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 (#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
Copy link
Contributor Author

filmil commented Nov 15, 2023

Also see #25

filmil added a commit to filmil/private_comments that referenced this issue Nov 17, 2023
@filmil
feat: adds build for Linux
cdcaf07 
This change adds some affordances for building private_comments
under Linux.  Since I could not guarantee that every builder would
have all the necessary tools for building, I wrapped the build
process into a docker environment.

This allows an almost trivial build command line:

```
./linux/make.release.sh
```

You need to have docker installed for this to be possible, but it might
not be a big concession to make for the ease of building. This still
requires root access to run Docker (sigh), but doesn't require you to
install a whole new runtime if you are only interested in running the
binaries.

I used shell scripts for this purpose to follow the style established in
the repository, though I'd have prefered to use some `make` tool.

Isuses: masukomi#22
@filmil filmil mentioned this issue Nov 17, 2023
Merged
filmil added a commit to filmil/private_comments that referenced this issue Nov 17, 2023
@filmil
feat: adds build for Linux
a245091 
This change adds some affordances for building private_comments
under Linux.  Since I could not guarantee that every builder would
have all the necessary tools for building, I wrapped the build
process into a docker environment.

This allows an almost trivial build command line:

```
./linux/make.release.sh
```

You need to have docker installed for this to be possible, but it might
not be a big concession to make for the ease of building. This still
requires root access to run Docker (sigh), but doesn't require you to
install a whole new runtime if you are only interested in running the
binaries.

I used shell scripts for this purpose to follow the style established in
the repository, though I'd have prefered to use some `make` tool.

Issues: masukomi#22
@masukomi
Copy link
Owner

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
Copy link
Contributor Author

filmil commented Nov 17, 2023

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.

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@masukomi @filmil