Bug 16505 - rebuild_zebra.pl skips updates if -x is passed
Summary: rebuild_zebra.pl skips updates if -x is passed
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: Oldversions
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 16506
  Show dependency treegraph
 
Reported: 2016-05-12 13:20 UTC by Tomás Cohen Arazi
Modified: 2018-12-16 19:42 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed (5.39 KB, patch)
2016-05-13 15:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed (5.47 KB, patch)
2016-05-16 13:19 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed (5.60 KB, patch)
2016-05-16 16:06 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed (5.66 KB, patch)
2016-05-21 08:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16505: Make sure $as_xml will not be used later (816 bytes, patch)
2016-05-21 09:24 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 Tomás Cohen Arazi 2016-05-12 13:20:16 UTC

    
Comment 1 Tomás Cohen Arazi 2016-05-13 15:10:26 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-05-13 20:08:16 UTC
Minor: use q{} instead of qq{} if you do not need to interpolate variables.
Comment 3 Tomás Cohen Arazi 2016-05-13 22:23:23 UTC
(In reply to Jonathan Druart from comment #2)
> Minor: use q{} instead of qq{} if you do not need to interpolate variables.

Heh, yes. That's a leftover from the first approach, that changed things a bit more, so I delayed it for an enhancement bug. Good catch
Comment 4 Tomás Cohen Arazi 2016-05-16 13:19:39 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2016-05-16 16:06:32 UTC Comment hidden (obsolete)
Comment 6 David Cook 2016-05-17 04:18:24 UTC
When did this issue appear? I think rebuild_zebra.pl is working fine in 3.20.3?
Comment 7 Jonathan Druart 2016-05-21 08:43:04 UTC
Created attachment 51674 [details] [review]
Bug 16505: <collection> is missing the marc namespace and updates fail if -x is passed

Using rebuild_zerba.pl with the -x option switch, produces an incorrect output in
terms of what our XSLTs expect for indexing. This patch introduces the right namespace information
on the exported records so indexing succeeds.

To test:
- On current master, have some records on your db
- Run:
  $ sudo koha-shell kohadev
  $ cd kohaclone
  $ misc/migration_tools/rebuild_zebra.pl -r -b -k -x
=> you will get a message like this:

NOTHING cleaned : the export /tmp/NL5ufjUfpp has been kept.

- Run
  $ less /tmp/NL5ufjUfpp/biblio/exported_records
=> FAIL: The first line looks like this

<?xml version="1.0" encoding="UTF-8"?><collection><record

- Now run:
  $ xsltproc \
     /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl \
     /tmp/NL5ufjUfpp/biblio/exported_records
=> FAIL: No output
- Apply the patch
- Run:
  $ misc/migration_tools/rebuild_zebra.pl -r -b -k -x
- Take a look at the result file:
  $ less /tmp/asdiouqwiue/biblio/exported_records
=> SUCCESS: The start of the file looks like this:
<?xml version="1.0" encoding="UTF-8"?><collection xmlns="http://www.loc.gov/MARC21/slim">

- Run:
  $ xsltproc \
     /etc/koha/zebradb/marc_defs/marc21/biblios/biblio-zebra-indexdefs.xsl \
     /tmp/asdiouqwiue/biblio/exported_records
=> SUCCESS: There is actually indexing data :-D
- Sign off :-D

Edit: I changed qq{} for q{} as suggested by Jonathan.

Sponsored-by: American Numismatic Society

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works as described following test plan
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2016-05-21 09:24:21 UTC
Created attachment 51679 [details] [review]
Bug 16505: Make sure $as_xml will not be used later
Comment 9 Kyle M Hall 2016-05-23 17:14:40 UTC
Pushed to master for 16.05, thanks Tomas!

NOTE: Only the first patch was pushed. The second warning patch no longer applies. An updated patch should probably be pushed.
Comment 10 Kyle M Hall 2016-05-23 17:30:12 UTC
Second patch has now been pushed!
Comment 11 Julian Maurice 2016-05-25 08:54:59 UTC
This is weird, bug 16506 depends on the first patch of this bug, but the 2nd patch of this bug depends on bug 16506. They have been pushed to master in this order: 16505 (1st) -> 16506 (all) -> 16505 (2nd).
I think the 2nd patch here belongs to bug 16506.
Comment 12 Julian Maurice 2016-05-25 08:55:59 UTC
First patch pushed to 3.22.x, will be in 3.22.7
Comment 13 Tomás Cohen Arazi 2016-05-25 13:41:19 UTC
(In reply to Julian Maurice from comment #11)
> This is weird, bug 16506 depends on the first patch of this bug, but the 2nd
> patch of this bug depends on bug 16506. They have been pushed to master in
> this order: 16505 (1st) -> 16506 (all) -> 16505 (2nd).
> I think the 2nd patch here belongs to bug 16506.

I take the blame for the circular bug dependency :-D
And you are right about Jonathan's followup.