I was trying to figure out why the POS functionality is giving us a 'not found' in our 20.05.00 installation. I noticed the directory is not listed in Makefile.PL, but I believe it needs to be.
My coworker confirmed directory and .pl files are missing.
Created attachment 108781 [details] [review] Bug 26265: Add pos directory to the Makefile This patch adds the /pos directory to the Makefile so that pos gets properly mapped at make and install time.
Created attachment 108783 [details] [review] Bug 26265: Add pos directory to the Makefile This patch adds the /pos directory to the Makefile so that pos gets properly mapped at make and install time. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I am sorry, I can't test the packaging step for this patch. We have a history of this problem with other newly added modules: http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=e57b3a6c4d9bb6319f038909832281dd4a9e39e5
http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=99f3121ff874fdc38aa607ca0db5b16436dd16ca (rotating collections) There Tomas noted that the patch should fix packages too.
http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=07ba06a5af4ec9bfea63769bf6162f0f8aec0a94 (plugins) Also mentions it as a fix for packages. Wow, we were really bad at this ;)
Sounds like one we should perhaps have a QA script test for...
https://gitlab.com/koha-community/qa-test-tools/-/issues/36
Or remove Makefile.PL ? :)
(In reply to Marcel de Rooy from comment #9) > Or remove Makefile.PL ? :) And do what instead? It appears to be used by any installation type we currently have. Can you please PQA?
Created attachment 108815 [details] [review] Bug 26265: Add test for missing directory in Makefile.PL
Created attachment 108816 [details] [review] Bug 26265: Add missing xt dir
(In reply to Martin Renvoize from comment #7) > Sounds like one we should perhaps have a QA script test for... No, it should be a Koha test IMO. I had a try but it does not pass, no idea why. The code is trivial!
We should exclude .git and docs as well. But why xt and patroncards are listed? They are present in the Makefile. I am missing something obvious (getting hit by Friday afternoon)
Created attachment 108819 [details] [review] Bug 26265: Add missing xt dir
grr.. I have no idea why the regex match fails for some of those cases.. it's really odd.
Can we have the simple patch pushed and fix the tests later? I believe the stables are pending release any moment now.
Having a look
(In reply to Jonathan Druart from comment #14) > We should exclude .git and docs as well. > But why xt and patroncards are listed? They are present in the Makefile. I > am missing something obvious (getting hit by Friday afternoon) I was distracted by the slash in the directory path, but the real problem seems to be the g option of the regex which makes it match multiple times in iteration. Remove it and it works.. We should also add the bin directory.
Created attachment 108967 [details] [review] Bug 26265: Remove g option from regex, add few dirs Adding .git, bin and docs Test plan: Run the test again Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
No, I realize that bin is something peculiar to my container setup..
Created attachment 108968 [details] [review] Bug 26265: Add pos directory to the Makefile This patch adds the /pos directory to the Makefile so that pos gets properly mapped at make and install time. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 108969 [details] [review] Bug 26265: Add test for missing directory in Makefile.PL Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 108970 [details] [review] Bug 26265: Add missing xt dir Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 108971 [details] [review] Bug 26265: (QA follow-up) Remove g option from regex, add few dirs Adding .git. Using a regex. If we make it a bit smarter, we do not need most dirs listed. Test plan: Run the test again Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 20.11, thanks to everybody involved!
Created attachment 109071 [details] [review] Bug 26265: add a plan for tests Tests were run but no plan was declared and done_testing() was not seen.
Last patch pushed to master for 20.11.
The entire patchset has been backported to 20.05.x
missing dependencies, not backported to 19.11.x