ad-oliviero / uwufetch

A meme system info tool for Linux, based on nyan/uwu trend on r/linuxmasterrace.
GNU General Public License v3.0
747 stars 48 forks source link

Force creation of directories (useful for packaging) #124

Closed kescherCode closed 3 years ago

kescherCode commented 3 years ago

In packaging systems (cough such as the one on Arch Linux cough), directories such as $pkgdir/usr/{bin,lib,share/man/man1} do not exist yet. Therefore, as it currently stands, there are errors like this:

cp: cannot create regular file '/tmp/uwufetch/pkg/uwufetch/usr/bin/uwufetch': No such file or directory

Since /usr/bin inside of /tmp/uwufetch/pkg/uwufetch might not yet exist, we have to create /usr first and /usr/bin afterwards. Luckily, mkdir has a -p switch, which does this for us.

q60 commented 3 years ago

i recommend replacing existing PKGBUILD with this

# Maintainer: Darkpelz <lukeh@outlook.my>
# Maintainer: TheDarkBug <adrianoliviero23@gmail.com>
pkgname=uwufetch
pkgver=1.5
pkgrel=3
pkgdesc='A meme system info tool for Linux, based on nyan/uwu trend on r/linuxmasterrace.'
arch=('any')
url="https://github.com/q60/$pkgname"
license=('GPL3')
makedepends=('gcc')
optdepends=('viu: Display distro logos as images'
            'lshw: Better GPU detection')
source=("$pkgname-$pkgver.tar.gz::$url/archive/$pkgver.tar.gz")
sha256sums=('3cdbfc12dc08e6b36b1844644c72e78a95d331ec2f50dfc7b00c7b6a73803034')

build() {
    cd "$pkgname-$pkgver"
    cc -O3 -o $pkgname $pkgname.c
}

package() {
    cd "$pkgname-$pkgver"
    install -Dm755 "$pkgname" "$pkgdir/usr/bin/$pkgname"
    install -Dm644 "$pkgname.1.gz" "$pkgdir/usr/share/man/man1/$pkgname.1.gz"
}

since PKGBUILD is used only in Arch and derived systems, there's no need in Makefile at all, because paths are all the same, unlike with BSD/Darwin.

kescherCode commented 3 years ago

@q60 My counterpoint to that would be that simply running make install (and similar commands) is a very standard way to install in packaging systems - a lot of PKGBUILDs do it this way, and I imagine a lot of packaging systems for other distros do this as well. If there's a Makefile, making a PKGBUILD do the same steps an existing Makefile is supposed to do is unneeded duplication, in my opinion.

q60 commented 3 years ago

@kescherCode and my counterpoint to yours would be that in this specific case there's no need to reinvent the wheel using a number of mkdir -p to tame that Makefile, it would be better to use standard install command with no excess stuff like making directories. using cp-and-mkdir approach is discouraged in PKGBUILD, it's recommended to use install to make code clean and avoid any side effects.

q60 commented 3 years ago

hmm, does your solution with including mkdir right into the Makefile work with existing PKGBUILD?

kescherCode commented 3 years ago

Yes.

kescherCode commented 3 years ago

Actually, when it comes to cp and mkdir, I suppose we should really use install in the Makefile. Therefore, closing this.

q60 commented 3 years ago

i apologize, i didn't notice that this was actually a pull request, not an issue, so i thought you suggested to add mkdir to PKGBUILD

q60 commented 3 years ago

Actually, when it comes to cp and mkdir, I suppose we should really use install in the Makefile. Therefore, closing this.

this makefile targets not only Linux, but BSD and Darwin too. BSD install has different syntax, so i find your solution right

q60 commented 3 years ago

re-open it, please

kescherCode commented 3 years ago

Alright then :)

q60 commented 3 years ago

@kescherCode as you are author of this pull request, you can make another change to Makefile - remove build rule from install rules, so installing won't rebuild already built binary

kescherCode commented 3 years ago

done

ad-oliviero commented 3 years ago

@q60 I used your PKGBUILD in the latest aur commit (to fix #125), but how should I use install for directories?

kescherCode commented 3 years ago

@TheDarkBug The PKGBUILD change is not neccesary with the updated Makefile.

Also, install creates non-existing directories, but as was mentioned, this is only useful for Linux (as other platforms have install binaries with different syntax).

The intent of this pull request was to change the Makefile so the PKGBUILD could stay the same as before, while fixing #125.

ad-oliviero commented 3 years ago

The PKGBUILD change is not neccesary with the updated Makefile.

I know, but a fix was needed for #125 and I chose to use this PKGBUILD, just because it worked

Also, install creates non-existing directories, but as was mentioned, this is only useful for Linux (as other platforms have install binaries with different syntax).

I am using install only for the aur packages, so this should not be a problem

The intent of this pull request was to change the Makefile so the PKGBUILD could stay the same as before, while fixing #125.

Should I release a new version for a better installation process?

kescherCode commented 3 years ago

@TheDarkBug I think it would be optimal to release a new version that includes this Makefile and to revert the PKGBUILD to the previous commit on the AUR again.

q60 commented 3 years ago
  1. release 1.6
# Maintainer: Darkpelz <lukeh@outlook.my>
# Maintainer: TheDarkBug <adrianoliviero23@gmail.com>
pkgname=uwufetch
pkgver=1.6
pkgrel=1
pkgdesc="A meme system info tool for Linux, based on nyan/uwu trend on r/linuxmasterrace."
arch=('any')
url="https://github.com/TheDarkBug/$pkgname"
license=('GPL3')
makedepends=("gcc")
optdepends=("viu: Display distro logos as images"
            "lshw: Better GPU detection")
source=("$pkgname-$pkgver.tar.gz::https://github.com/TheDarkBug/uwufetch/archive/$pkgver.tar.gz")
sha256sums=("1.6 sha256sum goes here")

build() {
    cd "$pkgname-$pkgver"
    make build
}

package() {
    cd "$pkgname-$pkgver"
    make DESTDIR="$pkgdir" install
}
kescherCode commented 3 years ago

pretty much.

ad-oliviero commented 3 years ago

Done, now I can close #125

q60 commented 3 years ago

nice!

image