Bug 17265

Summary: Make koha-create and koha-dump-defaults less greedy
Product: Koha Reporter: Magnus Enger <magnus>
Component: PackagingAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: arouss1980, julian.maurice, katrin.fischer, kyle, mirko, mtj
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 17265 - Make koha-create-defaults less greedy
Bug 17265 - Make koha-create-defaults less greedy
Bug 17265 - Make koha-create-defaults less greedy

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 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 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