Bug 17265 - Make koha-create and koha-dump-defaults less greedy
Summary: Make koha-create and koha-dump-defaults less greedy
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-07 11:54 UTC by Magnus Enger
Modified: 2018-06-04 20:10 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 17265 - Make koha-create-defaults less greedy (2.44 KB, patch)
2016-09-08 12:24 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 17265 - Make koha-create-defaults less greedy (2.51 KB, patch)
2016-09-10 08:08 UTC, Andreas Roussos
Details | Diff | Splinter Review
Bug 17265 - Make koha-create-defaults less greedy (2.56 KB, patch)
2016-12-30 18:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Magnus Enger 2016-09-07 11:54:29 UTC
koha-dump-defaults runs mysqldump and then pipes the result to sed, to replace every occurence of the site name with the placeholder __KOHASITE__

  mysqldump --databases --host="$mysqlhost" --user="$mysqluser" \
    --password="$mysqlpass" "$mysqldb" |
    sed "s/$name/__KOHASITE__/g"

I had used the silly name "data" as the site name for my "template instance", and this resulted in innocent things like 

  -- Dumping __KOHASITE__ for table `zebraqueue`

and bad things like 

  'Edit catalog (Modify bibliographic/holdings __KOHASITE__)'

or this, in biblioitems.marcxml:

  <__KOHASITE__field>

All told there were 7235 occurences of __KOHASITE__ in my rather small SQL file. 

As suggested by drojf on IRC, I *think* this would be fixed if we replaced "koha_$name" __INSTANCENAME__, instead of just $name.
Comment 1 Magnus Enger 2016-09-07 11:56:30 UTC
(In reply to Magnus Enger from comment #0)
> As suggested by drojf on IRC, I *think* this would be fixed if we replaced
> "koha_$name" __INSTANCENAME__, instead of just $name.

That should of course be __KOHASITE__, not __INSTANCENAME__.
Comment 2 Magnus Enger 2016-09-07 12:15:49 UTC
...and we would need to change line 744 of koha-create accordingly:

        sed "s/__KOHASITE__/$name/g" |
Comment 3 Magnus Enger 2016-09-08 12:24:41 UTC Comment hidden (obsolete)
Comment 4 Andreas Roussos 2016-09-10 08:02:07 UTC
Apologies for being pedantic, but may I suggest a small change to the bug title?
'koha-create-defaults' is not a valid command. In order to find this bug in
future BZ/git searches for 'koha-create' or 'koha-dump-defaults' I think the
title should be:

"Make koha-create and koha-dump-defaults less greedy"
Comment 5 Andreas Roussos 2016-09-10 08:08:30 UTC
Created attachment 55458 [details] [review]
Bug 17265 - Make koha-create-defaults less greedy

koha-create-defaults substitutes the instance name with the placeholder
__KOHASITE__ in the SQL file that it creates. If the instance name is
something common, like "data", this means that way too many substitutions
will be made, and when koha-create uses the SQL file as a template, broken
data can be the result.

The solution in this patch was suggested by drojf on IRC.

To test:
- Create an instance called "data"
- Run "sudo koha-dump-defaults data > test.sql"
- Take a look at test.sql and verify there are way too many occurences
  of __KOHASITE__, like "-- Dumping __KOHASITE__ for table `accountlines`"
- Apply the patch
- Copy the resulting koha-create and koha-dump-defaults to somewhere
  running off the packages
- Run "sudo koha-dump-defaults data > test.sql" again and verify there
  are only 4 occurrences of __KOHASITE__, in places that make sense
- Create an instance with something like:
  $ gzip test.sql
  $ sudo koha-create --create-db --defaultsql test.sql.gz test17265
- Verify that you have a working Koha install

Signed-off-by: Andreas Roussos <arouss1980@gmail.com>
Works as expected.
Comment 6 Katrin Fischer 2016-09-10 08:33:23 UTC
Changed the bug title according to Andreas' suggestion. :)
Comment 7 Andreas Roussos 2016-09-16 19:43:28 UTC
Sorry, forgot to set Status to 'Signed Off', setting it now.
Comment 8 Kyle M Hall (khall) 2016-12-30 18:02:48 UTC
Created attachment 58540 [details] [review]
Bug 17265 - Make koha-create-defaults less greedy

koha-create-defaults substitutes the instance name with the placeholder
__KOHASITE__ in the SQL file that it creates. If the instance name is
something common, like "data", this means that way too many substitutions
will be made, and when koha-create uses the SQL file as a template, broken
data can be the result.

The solution in this patch was suggested by drojf on IRC.

To test:
- Create an instance called "data"
- Run "sudo koha-dump-defaults data > test.sql"
- Take a look at test.sql and verify there are way too many occurences
  of __KOHASITE__, like "-- Dumping __KOHASITE__ for table `accountlines`"
- Apply the patch
- Copy the resulting koha-create and koha-dump-defaults to somewhere
  running off the packages
- Run "sudo koha-dump-defaults data > test.sql" again and verify there
  are only 4 occurrences of __KOHASITE__, in places that make sense
- Create an instance with something like:
  $ gzip test.sql
  $ sudo koha-create --create-db --defaultsql test.sql.gz test17265
- Verify that you have a working Koha install

Signed-off-by: Andreas Roussos <arouss1980@gmail.com>
Works as expected.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall (khall) 2017-01-13 11:32:38 UTC
Pushed to master for 17.05, thanks Magnus!
Comment 10 Katrin Fischer 2017-01-15 16:53:28 UTC
This patch has been pushed to 16.11.x and will be in 16.11.02.
Comment 11 Julian Maurice 2017-01-19 14:43:48 UTC
Pushed to 3.22.x for 3.22.15
Comment 12 Mason James 2017-01-27 02:50:01 UTC
Pushed to 16.05.x, for 16.05.08 release