Bug 26265 - Makefile.PL is missing pos directory
Summary: Makefile.PL is missing pos directory
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low blocker (vote)
Assignee: Martin Renvoize
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-21 08:13 UTC by Katrin Fischer
Modified: 2020-09-09 03:49 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.04


Attachments
Bug 26265: Add pos directory to the Makefile (946 bytes, patch)
2020-08-21 08:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26265: Add pos directory to the Makefile (1004 bytes, patch)
2020-08-21 09:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26265: Add test for missing directory in Makefile.PL (1.86 KB, patch)
2020-08-21 13:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26265: Add missing xt dir (1.37 KB, patch)
2020-08-21 13:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26265: Add missing xt dir (858 bytes, patch)
2020-08-21 13:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26265: Remove g option from regex, add few dirs (1.05 KB, patch)
2020-08-24 08:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26265: Add pos directory to the Makefile (1.08 KB, patch)
2020-08-24 08:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26265: Add test for missing directory in Makefile.PL (1.95 KB, patch)
2020-08-24 08:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26265: Add missing xt dir (955 bytes, patch)
2020-08-24 08:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26265: (QA follow-up) Remove g option from regex, add few dirs (1.10 KB, patch)
2020-08-24 08:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 26265: add a plan for tests (640 bytes, patch)
2020-08-25 13:14 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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 2020-08-21 10:44:08 UTC
Sounds like one we should perhaps have a QA script test for...
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 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 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