Bug 26265

Summary: Makefile.PL is missing pos directory
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: Installation and upgrade (command-line installer)Assignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: blocker    
Priority: P5 - low CC: aleisha, jonathan.druart, lucas, m.de.rooy, martin.renvoize, mtj, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.04
Circulation function:
Attachments: Bug 26265: Add pos directory to the Makefile
Bug 26265: Add pos directory to the Makefile
Bug 26265: Add test for missing directory in Makefile.PL
Bug 26265: Add missing xt dir
Bug 26265: Add missing xt dir
Bug 26265: Remove g option from regex, add few dirs
Bug 26265: Add pos directory to the Makefile
Bug 26265: Add test for missing directory in Makefile.PL
Bug 26265: Add missing xt dir
Bug 26265: (QA follow-up) Remove g option from regex, add few dirs
Bug 26265: add a plan for tests

Description Katrin Fischer 2020-08-21 08:13:20 UTC
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.
Comment 1 Katrin Fischer 2020-08-21 08:25:14 UTC
My coworker confirmed directory and .pl files are missing.
Comment 2 Martin Renvoize (ashimema) 2020-08-21 08:25:36 UTC
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.
Comment 3 Katrin Fischer 2020-08-21 09:08:39 UTC
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>
Comment 4 Katrin Fischer 2020-08-21 09:11:55 UTC
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
Comment 5 Katrin Fischer 2020-08-21 09:13:38 UTC
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.
Comment 6 Katrin Fischer 2020-08-21 09:15:51 UTC
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 ;)
Comment 7 Martin Renvoize (ashimema) 2020-08-21 10:44:08 UTC
Sounds like one we should perhaps have a QA script test for...
Comment 8 Martin Renvoize (ashimema) 2020-08-21 10:54:06 UTC
https://gitlab.com/koha-community/qa-test-tools/-/issues/36
Comment 9 Marcel de Rooy 2020-08-21 12:37:18 UTC
Or remove Makefile.PL ? :)
Comment 10 Katrin Fischer 2020-08-21 12:46:59 UTC
(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?
Comment 11 Jonathan Druart 2020-08-21 13:10:38 UTC
Created attachment 108815 [details] [review]
Bug 26265: Add test for missing directory in Makefile.PL
Comment 12 Jonathan Druart 2020-08-21 13:10:42 UTC
Created attachment 108816 [details] [review]
Bug 26265: Add missing xt dir
Comment 13 Jonathan Druart 2020-08-21 13:11:20 UTC
(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!
Comment 14 Jonathan Druart 2020-08-21 13:12:57 UTC
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)
Comment 15 Jonathan Druart 2020-08-21 13:27:55 UTC
Created attachment 108819 [details] [review]
Bug 26265: Add missing xt dir
Comment 16 Martin Renvoize (ashimema) 2020-08-21 14:47:00 UTC
grr.. I have no idea why the regex match fails for some of those cases.. it's really odd.
Comment 17 Katrin Fischer 2020-08-24 06:35:43 UTC
Can we have the simple patch pushed and fix the tests later? I believe the stables are pending release any moment now.
Comment 18 Marcel de Rooy 2020-08-24 07:30:22 UTC
Having a look
Comment 19 Marcel de Rooy 2020-08-24 08:00:47 UTC
(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.
Comment 20 Marcel de Rooy 2020-08-24 08:05:55 UTC
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>
Comment 21 Marcel de Rooy 2020-08-24 08:24:04 UTC
No, I realize that bin is something peculiar to my container setup..
Comment 22 Marcel de Rooy 2020-08-24 08:38:33 UTC
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>
Comment 23 Marcel de Rooy 2020-08-24 08:38:39 UTC
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>
Comment 24 Marcel de Rooy 2020-08-24 08:38:44 UTC
Created attachment 108970 [details] [review]
Bug 26265: Add missing xt dir

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 25 Marcel de Rooy 2020-08-24 08:38:52 UTC
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>
Comment 26 Jonathan Druart 2020-08-24 10:34:01 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 27 Jonathan Druart 2020-08-25 13:14:59 UTC
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.
Comment 28 Jonathan Druart 2020-08-25 13:16:50 UTC
Last patch pushed to master for 20.11.
Comment 29 Lucas Gass (lukeg) 2020-09-02 14:44:34 UTC
The entire patchset has been backported to 20.05.x
Comment 30 Aleisha Amohia 2020-09-09 03:49:56 UTC
missing dependencies, not backported to 19.11.x