-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[htdocs/web-src] Include build of htdocs files into configure/make #962
Conversation
Sounds very good to me. Autotools is black magic to me, so I don't know how much I can help, but I will take a look at it and also test it on a few platforms. |
Here's a bit of feedback on this after I looked at a few "special" platforms. OpenWrt: FreeBSD 12.1: It would also be good if we could somehow get it tested on MacOS. Regarding the Makefile itself, shouldn't "cp" be "$(INSTALL)" or something like that? |
The result of
Will have to try this out see what is missing/wrong ... will report back.
I honestly don't know, what the right approach is. I will try with "$(INSTALL)". |
Ok maybe my understanding of how it works is lacking then - when I run "make dist-xz" then npm is not invoked (unlike gperf and antlr), so instead the web interface (which is otherwise the output of npm) is copied from |
Add a Makefile into web-src that takes care of building the forked-daapd player web interface. It only builds the web interface if the npm binary is installed on the build system. Otherwise the prebuild files under the dist directory are used.
Did some more trial and error ... and change it to not directly include the Makefile but instead use autoconf to create the web-src/Makefile from the Makefile.in file. It builds for me now on FreeBSD. This is based on: https://www.gnu.org/software/automake/manual/html_node/Third_002dParty-Makefiles.html (and by looking into the other automake generated Makefile.in files in forked-daapd) Regarding MacOS, maybe we could use Github actions for this? If you have some free time @ejurgensen, i'll appreciate if you could give this another try. |
The I also added some changes to |
I think using Github actions for MacOS is a good idea, so I tried adding such a workflow file. However, it seems there is some Github bug. It keeps saying that actions are not enabled, even though I did enable them. Looks like other people have the same issue: https://github.community/t5/GitHub-Actions/Unable-to-enable-Actions-for-this-repository/td-p/36260 Maybe it works in your fork? |
I updated the master branch in my fork and Github actions are working: https://github.com/chme/forked-daapd/actions (though the MacOS build fails with a permission problem installing antlr3) |
Github support was able to help get the actions working, so that is pretty nice. I tried building the branch again on OpenWrt with no npm, and it fails with:
But perhaps that was also the intention? I'm unsure what your plan was when building from git and npm isn't present. If I can make a release with e.g. "make dist-gzip" on a machine with npm, and then build from the tarball on another machine without npm I would think it is fine. Then it would be like antlr3/gperf. |
I’m looking forward latest forked-daapd packages release With web interface. |
@chme I'm not sure about the status of this PR, but if you think it is ok you are welcome to merge it |
There are still problems I need to fix. |
Closing in favor of #1439 |
@ejurgensen this is my attempt at including the build of the web interface into the forked-daapd configure/make build.
I have to say, that my Makefile/autotools knowledge is really limited and i also did not find good examples that already have an npm built web interface included into the autotools toolchain.
What this PR does is:
web-src
that is included in the root Makefile.ammake
make dist-web
that creates aforked-daapd-htdocs-VERSION.tar.gz
file for the web interface dist filesmake dist
)There is probably a lot, that i am doing wrong ... so i would appreciate, if you can take a look at the changes and tell me if this is something we want and if yes, what is missing and needs to be changed/added/removed.