qsyncthingtray-git: Sync files among devices without a server

  • Syncthing: Syncs files among devices without the need of a server.

  • QSyncthinTray: Adds a tray icon that shows sync status.

  • qsyncthingtray-git: The brand new package with many bug fixes.

1 Like

Recommend you look at the plain qsyncthingtray as a reference of a more standard PKGBUILD structure. You have a lot of non-standard stuff in yours.

Thanks for the suggestion, but that's on purpose. Nevertheless you are welcome to pinpoint individual problems, and we can see if there is a real issue.

Anyway nothing should break the standard behaviour, as it's thoroughly tested to work well.

Please don't ignore the current guidelines in an effort to reinvent them.

https://wiki.archlinux.org/index.php/Arch_package_guidelines

I get the feeling you don't really want any criticism of your PKGBUILD, but here goes:


1. #! /bin/bash

This is not an executable and should not include a shebang.


4. Name="QSyncthingTray"
5. LowercaseName=$(echo "${Name,,}")
6. pkgname="${LowercaseName}-git"

You have introduced two new variables, neither of which follow the guidelines in using a leading _ to denote a required local variable, and which also calls a subshell. In this instance this is pointless. This can be improved with a more standard approach:

pkgname=qsyncthingtray-git
_pkgname=QSyncthingTray

10. arch=("x86_64")
11. url="https://github.com/sieren/QSyncthingTray"
12. license=("LGPL3")
13. makedepends=("cmake" "git")
14. depends=("qt5-location" "qt5-webengine" "startup-settings-git" "syncthing")

Unnecessary quoting of strings. arch=(x86_64) is just fine.


14. depends=("qt5-location" "qt5-webengine" "startup-settings-git" "syncthing")

Does this actually depend on another -git package? It's far better to depend on a released version which doesn't change and possibly break hourly/daily.


15. provides=("${LowercaseName}")
16. conflicts=("${LowercaseName}")

Now there's a standard pkgname variable it's far easier to just use:

provides=("${pkgname/-git}")
conflicts=("${pkgname/-git}")

17. source=("git+${url}.git" "${LowercaseName}.desktop")
18. md5sums=("SKIP" "SKIP")

The first item in the source array is an appropriate use of quoting and the existing variable.

A checksum should be provided for the .desktop file, and you should probably switch to at least sha256.


22. 	cd "${srcdir}/${Name}"

You're already in the $srcdir, so this can be just

	cd $_pkgname

No need to quote a plain string.


24. 	echo

This breaks the pkgver() function as it will always "return" a null string. Remove this line.


29. 	cd "${srcdir}/${Name}"
30.
31. 	mkdir "build"
32. 	cd "build"
33.
34. 	cmake ".."
35. 	make

Line 29 is the same as above, but given you're already in $srcdir and CMake's build directory can be anywhere this can be simplified to

mkdir build && cd build
cmake ../$_pkgname
make

40. 	mkdir --parents "${pkgdir}/usr/bin"
41. 	mkdir --parents "${pkgdir}/usr/share/pixmaps"
42. 	mkdir --parents "${pkgdir}/usr/share/applications"
43.
44. 	mv "${srcdir}/${Name}/build/${Name}" "${pkgdir}/usr/bin/${LowercaseName}"
45. 	mv "${srcdir}/${Name}/resources/images/Icon1024.png" "${pkgdir}/usr/share/pixmaps/${LowercaseName}.png"
46. 	mv "${srcdir}/${LowercaseName}.desktop" "${pkgdir}/usr/share/applications/${LowercaseName}.desktop"
47.
58. 	chmod +x "${pkgdir}/usr/bin/${LowercaseName}"

Use install to ensure permissions are correct and skip duplicated items. This can then be simplified to:

	install -D     "$srcdir"/build/$_pkgname "$pkgdir"/usr/bin/${pkgname/-git}
	install -Dm644 "$srcdir"/$_pkgname/resources/images/Icon1024.png "$pkgdir"/usr/share/pixmaps/${pkgname/-git}.png
	install -Dt    "$pkgdir"/usr/share/applications/ "$srcdir"/${pkgname/-git}.desktop 

I'm not going to argue about whether these changes address a "real issue". I'm simply stating my view as an experienced package maintainer.

Feel free to ask the aur-general mailing list for a PKGBUILD review.

2 Likes

Nah, I'm just skeptical. Not the same as unwilling.

I'm rather curious on experimenting how to have these conversations constructively.

Although most likely it's rather a matter of trust, trust that people do things for a reason. That if they have good attitude they don't need to be supervised all the time, that they will eventually realice themselves.

So rather than having endless discussions about the details, having discussions about the intended outcome. Then delegate the small decisions to whoever implements it.

Because if I supposedly chose this person because of them being competent for doing the job, why shouldn't now trust them on making small decisions?

But lets see, just for curiosity:

It makes the Pluma text editor highlight the code automatically. Plus it allows to easily test functions and variables individually, versus having to test all together by building the hole package on each test.

The need is just the variables not to conflict when sourcing, how to do it really doesn't matter.

I chose to use an uppercase letter instead because not having symbols on names makes the code somehow easier to read, as there is a clearer visual distinction between variables and operands.

I chose to quote all strings so not having to think about when not quoting could lead to unintended consequences, what in Bash happens on many unexpected situations.

True, but that software doesn't have a non git version.

But that would only involve the git version, doesn't it?

I guess it needs to involve any other package providing the software.

What benefit would bring that? Would that prevent file corruption or security concerns in practice?

I don't know, just asking.

makepkg seems to do many things unnoticed, like changing to the srcdir on each function. So I wanted the code to be explicit about which folder you are at each particular time, just for readability purposes.

No, it returns a string with a line jump at its end. Which doesn't seem to affect setting the pkgver variable, and prevents weird glitches when testing the pkgver function individually.

The purpose was to make code as easy to read as possible, versus as short as possible.

I simply thought that moving files instead of duplicating them made more sense, as moving is instant and copying takes time.

Also the original package seems to set the maximum permissions... even for pictures. Among other issues like providing the wrong license, or having different build instructions than documented upstream.

Some Arch developers are the rudest assholes I have ever met online. And philosophically they are the opposite than me, both technically and dealing with people.

I like automation, they like all manual.
I like delegation, they like micro-management.
I like experimentation, they like fully standard.
I like all jokes, they like all serious.
I like soft, they like hard.

That would be like explaining nudism to a prude.