ActivityWatch / aw-server-rust

High-performance implementation of the ActivityWatch server, written in Rust
Mozilla Public License 2.0
191 stars 55 forks source link

change makefile to default to user writable directory for install by default #351

Open skewballfox opened 1 year ago

skewballfox commented 1 year ago

related to #350 I think the Makefile should default to somewhere writeable by the user. /usr/local is owned by root and does not have write permissions by anyone else. There are two ways I can think of handling this

if you would rather leave the Makefile mostly as is and allow the user to override this you can use

PREFIX ?= $(/usr/local)

this will read the value of PREFIX from the environment if it is set, otherwise default to /usr/local.

Alternatively, if you don't mind changing the default you could use

ifeq ($(SUDO_USER),)
    PREFIX := $(HOME)/.local
else
    PREFIX := /usr/local
endif

the environment variable SUDO_USER should only be set if the current command is running with sudo

this would only do a system wide installation if running with sudo make install

I could submit a PR for either of these if you specify which one you prefer

ErikBjare commented 1 year ago

I think the second option is good. Would be happy to accept a PR :)

On Thu, Mar 16, 2023, 18:41 Joshua Ferguson @.***> wrote:

related to #350 https://github.com/ActivityWatch/aw-server-rust/issues/350 I think the Makefile should default to somewhere writeable by the user. /usr/local is owned by root and does not have write permissions by anyone else. There are two ways I can think of handling this

if you would rather leave the Makefile mostly as is and allow the user to override this you can use

PREFIX ?= $(/usr/local)

this will read the value of PREFIX from the environment if it is set, otherwise default to /usr/local.

Alternatively, if you don't mind changing the default you could use

ifeq ($(SUDO_USER),) PREFIX := $(HOME)/.local else PREFIX := /usr/local endif

the environment variable SUDO_USER should only be set if the current command is running with sudo

this would only do a system wide installation if running with sudo make install

I could submit a PR for either of these if you specify which one you prefer

— Reply to this email directly, view it on GitHub https://github.com/ActivityWatch/aw-server-rust/issues/351, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKXDOTJCAU432F555LIMJLW4NGGPANCNFSM6AAAAAAV5RBXZU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

johan-bjareholt commented 1 year ago

The first one is the more "classical" way to do it. Not saying it's the best, just that it's a very well-known pattern.

Also, regardless which version you choose you should use ?= for the prefix, so the user has the ability to override the prefix without modifying the Makefile.

On top of that, we have some hardcoded paths in aw-server-rust where to find the static web files, so using some other prefix will break the web-ui unless you do some additional change.

skewballfox commented 1 year ago

Also, regardless which version you choose you should use ?= for the prefix, so the user has the ability to override the prefix without modifying the Makefile.

On top of that, we have some hardcoded paths in aw-server-rust where to find the static web files, so using some other prefix will break the web-ui unless you do some additional change.

I addressed both in #353 Kind of messed up at first (didn't realize that && wasn't a thing in makefiles), but I reran make install and rust-server to confirm that they work.

right now the prefix setting is a little redundant because my make foo isn't good enough to figure out how to chain conditionals when one of those is meant to be an empty value. but it first checks if the user is root, then checks if one of the variables set by sudo is present. If there is anything I should do to update that, please let me know