search

jcorporation / myMPD

myMPD is a standalone and mobile friendly web mpd client with a tiny footprint and advanced features.
https://jcorporation.github.io/myMPD/
GNU General Public License v3.0
418 stars 65 forks source link

PKGBUILD should follow Arch Linux Web application package guidelines #131

Closed CultofRobots closed 5 years ago

CultofRobots commented 5 years ago

In order for myMPD to be submitted to AUR the PKGBUILD should follow Web application package guidelines: https://wiki.archlinux.org/index.php/Web_application_package_guidelines

Basically myMPD should be installed in /usr/share/webapps/mympd instead of /usr/share/mympd, and the config directory should be installed in /etc/webapps/mympd.

I've been meaning to get to this myself, I just haven't gotten to it yet. I'll post mine when/if I do, but if someone else gets to it first then great.

Secondly, there doesn't need to be "if/else" statements in the build and package functions of the PKGBUILD? When running makepkg from this PKGBUILD you are downloading a specific version of myMPD as stated in source=. This is not a PKGBUILD that builds from the latest git commits. It's specific, so there isn't any need to check directory structure. You either compressed and named the .tar.gz one way or you compressed and named it the other way and the PKGBUILD should reflect that. All that "if/else" is just noise. If you look at my PKGBUILD here I don't even need the [ -d release ] || mkdir release check. I just keep forgetting to take it out.

If you want a PKGBUILD that builds from the latest git or a different source, you need a different PKGBUILD with a different source and a pkgver function if source is from git (e.g. ympd-git PKGBUILD). PKGBUILDs aren't "one size fits all".

If I get a chance I'll post a git PKGBUILD later today.

CultofRobots commented 5 years ago

Here's a git PKGBUILD: myMPD-git-PKGBUILD.zip

extract zip. cd into directory and run makepkg -s -p myMPD-git-PKGBUILD

PKGBUILD pulls from git and updates version to the latest master commit.

jcorporation commented 5 years ago

myMPD in AUR would be greate.

The if/else statements are inserted to enable package building through the OpenSUSE build service. In this build environment the get from github fails and I don't want to support more than one PKGBUILD file. Eventually it is better to maintain an extra PKGBUILD for this setup.

Web application package guidelines: I changed the CMakeLists.txt to use /usr/share/webapps/mympd and /etc/webapps/mympd, if it detects a environment variable ARCHLINUX set to true. I changed my PKGBUILD appropriate. The changes are not tested, because I have no direct access to a Archlinux system.

CultofRobots commented 5 years ago

I'll test them tomorrow or Saturday if I get some time. Thanks.

CultofRobots commented 5 years ago

It builds properly, but doesn't run properly.

It gets stuck looking for settings in /etc/mympd. Once the config is moved to /etc/mympd and coverextractlib path is updated everything seems to run fine.

jcorporation commented 5 years ago

Thanks for testing. I will fix this two issues after the weekend.

jcorporation commented 5 years ago

The last commit should fix your issue.

CultofRobots commented 5 years ago

I'll test it as soon as I get a chance. Hopefully later today. Thanks.

CultofRobots commented 5 years ago

Everything builds and works fine. You just need to change the paths from /etc/mympd to /etc/webapps/mympd in archlinux.install.

CultofRobots commented 5 years ago

issues from the latest build that I missed due to lack of coffee:

queue and last played don't display any tracks the queue control glyph doesn't work on the browse page.

jcorporation commented 5 years ago

Both should be now fixed.

CultofRobots commented 5 years ago

nothing in journal control running in debug.

webconsole output is: playlist glyph

    v redacted.sitename/js/bootstrap-native-v4.min.js:24```
last played:
```TypeError: d is undefined mympd.min.js:124:260
    t redacted.sitename/js/mympd.min.js:124
    parseLastPlayed redacted.sitename/js/mympd.min.js:174
    onreadystatechange redacted.sitename/js/mympd.min.js:256

I'll build the latest commit

CultofRobots commented 5 years ago

Content Security Policy: The page’s settings blocked the loading of a resource at eval (“script-src”). Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”).

that's probably what's doing it. You also have a typo in the initialization page, btw. "myMMPD"

EDIT: Issue looks to be in ./myMPD/src/web_server.c

jcorporation commented 5 years ago

The error comes from the updated bootstrap.native plugin. The issue is: https://github.com/thednp/bootstrap.native/issues/233. I implemented a workaround.

The Content Security Policy Warning should not appear and doesn't appear in my setup. Which browser do you use?

CultofRobots commented 5 years ago

I should note, the problem is in firefox. Chrome/chromium seem to work fine.

If that's the workaround in web_server.c lines 317-320 it's not accounting for inline or eval. You may need to add 'unsafe-inline' and 'unsafe-eval' to script-src, or add nounce/hashes (which would be better).

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src

again, everything seems to be working properly in chromium but I'm almost certain the CSP is the problem in firefox.

EDIT: and the last build I installed was for standard linux install, not arch specific. I get the problem on both build types.

jcorporation commented 5 years ago

I opened a new issue for the content security policy issue. If the PKGBUILD is now ok, please close this issue.

CultofRobots commented 5 years ago

PKGBUILD is fine.