domjudge
domjudge copied to clipboard
DOMjudge
Put `lib/vendor` close to the webapp
The files have nothing to do with binaries on the system but are only relevant for executable code from DOMjudge (which has no clearly defined place in FHS).
This was discussed in a private channel with the maintainers: https://domjudge-org.slack.com/archives/GJ2JX7B7X/p1709760201078489?thread_ts=1709540681.476949&cid=GJ2JX7B7X
The configure file is not cleaned up yet to clearly show this choice in the next minor release. If there is no new discussion this can be changed (as in removed) in the next major release.
I checked in a local container and it seems to work but I rather first discuss if this is the proper fix to handle this change in FHS.
I agree with the change as it's an improvement. I do however think we should just flatten it further: I perceive no useful split between "libdir" and "datadir" as it's used currently in the domserver and I would just install all those things at the same root (which might be libdir).
The whole distinction between lib and share in FHS is dated and quickly becoming irrelevant (if it was ever relevant in the first place..), imho.
I think the RH effect of the webapp ending up in /run/webapp and other parts of the app under /usr shows to me what we currently do is not what I'd expect.
I agree with the change as it's an improvement. I do however think we should just flatten it further: I perceive no useful split between "libdir" and "datadir" as it's used currently in the domserver and I would just install all those things at the same root (which might be libdir).
I'm fine with implementing but can you state where you want those things to endup (so quote me and just fill the right values)
# * documentation.......: /usr/local/share/doc/domjudge
# * domserver...........:
# - bin..............: /usr/local/bin
# - etc..............: /usr/local/etc/domjudge
# - lib..............: /usr/local/lib/domjudge
# - libvendor........: /usr/local/share/domjudge/lib/vendor
# - log..............: /usr/local/var/log/domjudge
# - run..............: /usr/local/var/run/domjudge
# - sql..............: /usr/local/share/domjudge/sql
# - tmp..............: /tmp
# - webapp...........: /usr/local/share/domjudge/webapp
# - example_problems.: /usr/local/share/domjudge/example_problems
# - database_dumps...: /usr/local/var/lib/domjudge/db-dumps
Either way: we must make sure that the system works with any randomly set path for paths which are configurable. So if someone configures libvendordir and webappdir like this (as is currently possible):
# * domserver...........: # - lib..............: /usr/local/lib/domjudge # - libvendor........: /my/weird/directory/lib/vendor # - webapp...........: /random/other/dir/share/domjudge/webapp
then that must work, or it must not be possible to configure these paths.
If I have some time, I'm actually planning to write some build tests, that check this among other things.
Proposal: remove libvendordir setting and put it in
webapp/vendor
This would also remove a customization we need to do after every Symfony upgrade, so I would be in favor.
then that must work, or it must not be possible to configure these paths.
I propose to go for that 2nd option, I think spending your developer time on this (as you're the expert on this together with @thijskh IMHO) is not worth it. I personally prefer that that time is spend on other code (as I don't really see the usecase to split here).
Probably this breaks the CI still as @nickygerritsen mentioned a file which also needs changes.
If we do this, I would just move the thing alltogether, also in composer.json for example. Then libvendor can be completely removed from any configure scripts or makefiles, since it is already in the right place.
If we do this, I would just move the thing alltogether, also in
composer.jsonfor example. Then libvendor can be completely removed from any configure scripts or makefiles, since it is already in the right place.
Ok. I had the impression that in the past there were some other programs depending on this, but if that's not the case anymore, then sure.
If we do this, I would just move the thing alltogether, also in
composer.jsonfor example. Then libvendor can be completely removed from any configure scripts or makefiles, since it is already in the right place.Ok. I had the impression that in the past there were some other programs depending on this, but if that's not the case anymore, then sure.
So you're also fine with just removing this completely from configure.ac? It would be much simpler in that case if we don't display it there at all but that might be BC for people who had the opinion that those vendor files should be somewhere else.
It seems highly unlikely to me that there's any deployer counting on this being configurable.
If we do this, I would just move the thing alltogether, also in
composer.jsonfor example. Then libvendor can be completely removed from any configure scripts or makefiles, since it is already in the right place.Ok. I had the impression that in the past there were some other programs depending on this, but if that's not the case anymore, then sure.
So you're also fine with just removing this completely from
configure.ac? It would be much simpler in that case if we don't display it there at all but that might beBCfor people who had the opinion that thosevendorfiles should be somewhere else.
Yes, I think we must then remove it from configure.ac: if it were still possible to modify there, then things would break.
If we do this, I would just move the thing alltogether, also in
composer.jsonfor example. Then libvendor can be completely removed from any configure scripts or makefiles, since it is already in the right place.
@nickygerritsen it seems that it would put it next to webapp, not inside webapp. So we keep the problem for the copying for everything in webapp/public, did I do something wrong or are those indeed the proper locations (/vendor & /webapp)?
Hmmm maybe we should move the composer.json to webapp as well? Then it should be standard again
Hmmm maybe we should move the composer.json to webapp as well? Then it should be standard again
That would fix 1 issue yes, not sure if it introduces other ones.