bakkeby / dwm-flexipatch

A dwm build with preprocessor directives to decide which patches to include during build time
MIT License
1.18k stars 238 forks source link

Wrong xsessions directory in Makefile #317

Open Vinschers opened 1 year ago

Vinschers commented 1 year ago

I believe there is a typo in the Makefile when copying dwm.desktop to the xsessions directory. As stated in the ArchWiki, the default xsessions directory is /usr/share/xsessions, but the current Makefile copies dwm.desktop to ${DESTDIR}${PREFIX}/share/xsession (there is a s missing at the end).

Furthermore, I think it would be benefitial if the destination directory wasn't defined by the DESTDIR and PREFIX variables. Personally, I change the PREFIX inside config.mk to ~/.local, in order to install dwm locally. But, by doing so, the default xsessions directory ends up empty.

bakkeby commented 1 year ago

Yes but no but yes but no but yes but no but yes I agree on all points.

${DESTDIR}${PREFIX}/share/xsession (there is a s missing at the end).

That is indeed a typo.

the default xsessions directory is /usr/share/xsessions,

True. I have also noticed that the bspwm guys install their .desktop file under /usr/local/share/xsessions/. Both places are valid and should be picked up by display/login managers.

As for the difference between /usr/ and /usr/local/ I found this reference [ref]:

The original idea behind '/usr/local' was to have a separate ('local') '/usr' directory on every machine besides '/usr', which might be just mounted read-only from somewhere else. It copies the structure of '/usr'. These days, '/usr/local' is widely regarded as a good place in which to keep self-compiled or third-party programs. The /usr/local hierarchy is for use by the system administrator when installing software locally. It needs to be safe from being overwritten when the system software is updated. It may be used for programs and data that are shareable amongst a group of hosts, but not found in /usr. Locally installed software must be placed within /usr/local rather than /usr unless it is being installed to replace or upgrade software in /usr.

Then we have:

Personally, I change the PREFIX inside config.mk to ~/.local, in order to install dwm locally.

I like the general idea. So with dwm.desktop in /usr/share/xsessions/ users will be launching /usr/local/bin/dwm by default, but this can be overridden by having dwm installed under ~/.local/bin/dwm.

In the unlikely scenario that you have more than one people using dwm and sharing the same machine then they can run their separate builds of the window manager as needed.

I presume that for the PREFIX variable you put the full path to the .local directory rather than using ~ as compiling with sudo would put the files under the root directory. Alternatively one would have to add implicit sudo calls before the commands that deal with the dwm.desktop file and always run make without sudo.

bakkeby commented 1 year ago

I am wondering if it should it be /usr/share/xsessions/dwm.desktop or /usr/local/share/xsessions/dwm.desktop, perhaps it doesn't really matter.

Proposed change:

diff --git a/Makefile b/Makefile
index 01ea136..f309ae0 100644
--- a/Makefile
+++ b/Makefile
@@ -65,13 +65,13 @@ endif
        mkdir -p ${DESTDIR}${MANPREFIX}/man1
        sed "s/VERSION/${VERSION}/g" < dwm.1 > ${DESTDIR}${MANPREFIX}/man1/dwm.1
        chmod 644 ${DESTDIR}${MANPREFIX}/man1/dwm.1
-       mkdir -p ${DESTDIR}${PREFIX}/share/xsession
-       cp -n dwm.desktop ${DESTDIR}${PREFIX}/share/xsession
-       chmod 644 ${DESTDIR}${PREFIX}/share/xsession/dwm.desktop
+       mkdir -p /usr/share/xsessions
+       cp -n dwm.desktop /usr/share/xsessions
+       chmod 644 /usr/share/xsessions/dwm.desktop

 uninstall:
        rm -f ${DESTDIR}${PREFIX}/bin/dwm\
                ${DESTDIR}${MANPREFIX}/man1/dwm.1\
-               ${DESTDIR}${PREFIX}/share/xsession/dwm.desktop
+               /usr/share/xsessions/dwm.desktop

 .PHONY: all options clean dist install uninstall
Vinschers commented 1 year ago

Thank you for your reply!

Both places are valid and should be picked up by display/login managers.

Maybe the xsessions directory should be a variable inside config.mk, with /usr/share/xsessions being its default value?

In the unlikely scenario that you have more than one people using dwm and sharing the same machine then they can run their separate builds of the window manager as needed.

That was exactly my reasoning. Honestly, I don't think such a scenario would happen any time soon, but I thought it would be good practice to keep everything I use inside ~.

Alternatively one would have to add implicit sudo calls before the commands that deal with the dwm.desktop file and always run make without sudo.

That is the approach I am trying to use. I just added the sudo calls inside the Makefile.

I am wondering if it should it be /usr/share/xsessions/dwm.desktop or /usr/local/share/xsessions/dwm.desktop, perhaps it doesn't really matter.

Personally, I think the default path should be /usr/share/xsessions/dwm.desktop to preserve the defaults, even though /usr/local/share/xsessions/dwm.desktop would make more sense.

Proposed change:

Would it be bad if you added the sudo calls inside the Makefile instead of hoping the users run sudo make install? I really don't know if this would be bad practice, but it might make things easier on the user side.

aramacs commented 1 year ago

All the yes bot no thing make me crazy :P So what should I do? I double checked and I have both dwm.desktop in usr/local and usr/share. Sorry for my confusion I'm a padawan

bakkeby commented 1 year ago

I think the above should solve this, let me know if you spot any problems.

aramacs commented 1 year ago

I'll try out that change in makefile on the weekend and paste it here my results. By the way, congratulations with that project. I tried out a lot of WM, but Dwm is the best for my weak machine.

speedie1337 commented 1 year ago

Using sudo here is a bad idea for a few reasons.

Hardcoding the path also breaks some distributions such as NixOS. I am not sure how users of that distribution get around it, but since the PREFIX and DESTDIR variables are common I'd imagine they override that somewhere.

I would suggest reverting the commit, but fixing the path.

bakkeby commented 1 year ago

@speediegq thanks for the feedback.

I would suggest reverting the commit, but fixing the path.

I agree. Overall this seems to be the simplest approach.

For people that are specifically interested in user specific installation of dwm I think it would be better to accompany that with a How To discussion page outlining what would need changing in config.mk and Makefile to achieve that.