numixproject / numix-gtk-theme

A modern flat theme with a combination of light and dark elements.
GNU General Public License v3.0
1.19k stars 227 forks source link

make install fails on *BSD systems #713

Open DarkKirb opened 6 years ago

DarkKirb commented 6 years ago

make install fails on FreeBSD 11.1 in scripts/utils.sh, because:

khurshid-alam commented 6 years ago

BSD version doesn't support cp -t !? Source ? But without -t flag it can break debian build for different debhelper versions which treats double-spaces/tabs differently.

DarkKirb commented 6 years ago

according to cp(1):

https://www.freebsd.org/cgi/man.cgi?query=cp&apropos=0&sektion=1&manpath=FreeBSD+11.2-RELEASE+and+Ports&arch=default&format=html

output when you try to pass -t screenshot_2018-06-21_08-22-03

I could edit my pull request (#714) to include support for both BSD and debhelper

colinhb commented 6 years ago

Can confirm that OpenBSD's cp and ln do not have the -t and -r flags - which GNU coreutils does. Changes to build on OpenBSD 6.3 are in the diff below - similar to pull request from @DarkKirb. I switched to #!/bin/sh instead of keeping the dependency on bash since I didn't see any bashisms at first glance and think it can be safely removed.

Not sure I understand concern from @khurshid-alam. Believe -t just re-arranges arguments and any space / tab issues could be resolved through quoting or reformatting this script, but I'm not familiar with debhelper.

--- utils.sh.backup     Tue Oct  2 11:09:52 2018
+++ utils.sh    Tue Oct  2 11:14:02 2018
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh

 do_install() {
        local GTKDIR GTK320DIR GTKVER INSTALL_DIR
@@ -12,8 +12,9 @@

        cp index.theme "${INSTALL_DIR}"

-       cp -rt "${INSTALL_DIR}" \
-                       assets gtk-2.0 metacity-1 openbox-3 xfce-notify-4.0 xfwm4 unity
+       cp -r \
+                       assets gtk-2.0 metacity-1 openbox-3 xfce-notify-4.0 xfwm4 unity \
+                       "${INSTALL_DIR}"

        for _DIR in "${GTKDIR}" "${GTK320DIR}"
        do
@@ -21,14 +22,15 @@

                mkdir -p "${_DIR}"

-               cp -t "${_DIR}" \
+               cp \
                        "${GTKVER}/gtk.css" \
                        "${GTKVER}/gtk-dark.css" \
                        "${GTKVER}/gtk.gresource" \
-                       "${GTKVER}/thumbnail.png"
+                       "${GTKVER}/thumbnail.png" \
+                       "${_DIR}"

                cd "${_DIR}"
-               ln -srf ../assets assets
+               ln -sf ${PWD}/../assets assets
                cd -
        done
 }
khurshid-alam commented 6 years ago

ln -srf is required for is us devs who compiles without running make (using ./utils install) again and again. Is -r not supported again in bsd? Other than that it looks good. I asked at debian. If they approves I will merge it. Pleae rebase and update the PR.

Munchotaur commented 6 years ago

Still failing on OpenBSD 6.3 sadly.

khurshid-alam commented 6 years ago

@Munchotaur Does it fail even with this pull ?

Munchotaur commented 6 years ago

Yes, still failing due to a directory not being found, I think (username removed from paths):

openbsd-vm$ sudo make install Password: rm -rf src/gtk-3.0/dist rm -f src/gtk-3.0/gtk.gresource rm -rf src/gtk-3.20/dist rm -f src/gtk-3.20/gtk.gresource rm -rf /home//git/numix-gtk-theme/dist scss --update --sourcemap=none src/gtk-3.0/scss:src/gtk-3.0/dist directory src/gtk-3.0/dist write src/gtk-3.0/dist/gtk-dark.css write src/gtk-3.0/dist/gtk.css scss --update --sourcemap=none src/gtk-3.20/scss:src/gtk-3.20/dist directory src/gtk-3.20/dist write src/gtk-3.20/dist/gtk-dark.css write src/gtk-3.20/dist/gtk.css glib-compile-resources --sourcedir=src/gtk-3.0 src/gtk-3.0/gtk.gresource.xml glib-compile-resources --sourcedir=src/gtk-3.20 src/gtk-3.20/gtk.gresource.xml scripts/utils.sh install /usr/local/share/themes/Numix scripts/utils.sh: not found *** Error 1 in /home//git/numix-gtk-theme (Makefile:38 'install')

colinhb commented 6 years ago

@Munchotaur My guess is that you do not have bash installed, and scripts/utils.sh: not found is from the interpreter directive in the first line of the script: #!/usr/bin/env bash. You could verify this by running which bash. If bash is correctly installed, it will return the path to bash, which on OpenBSD would be /usr/local/bin/bash, and we'd need to look for another solution.

If you instead get Command not found from which bash, I would suggest using pkg_add bash to install bash. You could uninstall it immediately after building the theme. Alternatively you could edit the script to use /bin/sh as the interpreter or create a link bash from somewhere in your path to /bin/sh (which I would recommend removing immediately after since that is dangerous in general - this version of this script just happens to be ok). Hope that's helpful.

In general I think the dependency on bash is fine. I would recommend adding it to the readme, since utils.sh won't run without it. I can do a PR next week. (On the move this week.)

Munchotaur commented 6 years ago

Progress of a sort! I hardwired the shebang in utils.sh to use /usr/local/bin/bash and the 'not found' error disappeared. Now it falls over at a bunch of 'cp' option differences from the GNU version (-t unknown) -- much like your earlier posts. I don't know offhand if OpenBSD uses broadly the same 'cp' as FreeBSD.

colinhb commented 6 years ago

@Munchotaur For two reasons it sounds like you're not testing against this PR:

  1. Switching the shebang to the path to bash should only work if you have bash installed. But, if bash is installed, then the directive in the PR (#!/usr/bin/env bash) should also work. However, the current shebang #!/bin/bash would error with not found even if bash were installed (wrong path for OpenBSD), and your change would fix that problem.

  2. The PR removes the -r and -t flags from script, so you shouldn't see those errors.

You can test the PR in a clean repo by doing something like:

$ git fetch origin pull/714/head:pr # 714 is the ID of the PR
$ git checkout pr

Then try to build as usual. Let me know if that helps.

Munchotaur commented 5 years ago

@colinhb -- thanks for the advice. It works!

colinhb commented 5 years ago

@colinhb -- thanks for the advice. It works!

Glad to hear it. Thanks for letting me know. Looks like PR fixes issue for FreeBSD and OpenBSD.