Merge lp://qastaging/~tplavcic/percona-xtrabackup/bug1156209-2.2 into lp://qastaging/percona-xtrabackup/2.2

Proposed by Tomislav Plavcic
Status: Merged
Approved by: Alexey Kopytov
Approved revision: no longer in the source branch.
Merged at revision: 5064
Proposed branch: lp://qastaging/~tplavcic/percona-xtrabackup/bug1156209-2.2
Merge into: lp://qastaging/percona-xtrabackup/2.2
Diff against target: 531 lines (+98/-319)
12 files modified
.bzrignore (+1/-1)
cmake/build_configurations/xtrabackup_release.cmake (+2/-0)
storage/innobase/xtrabackup/.bzrignore (+1/-1)
storage/innobase/xtrabackup/CMakeLists.txt (+1/-0)
storage/innobase/xtrabackup/doc/Makefile (+0/-143)
storage/innobase/xtrabackup/doc/make.bat (+0/-170)
storage/innobase/xtrabackup/doc/source/CMakeLists.txt (+80/-0)
storage/innobase/xtrabackup/utils/build-binary.sh (+1/-1)
storage/innobase/xtrabackup/utils/debian/control (+3/-1)
storage/innobase/xtrabackup/utils/debian/percona-xtrabackup.install (+1/-0)
storage/innobase/xtrabackup/utils/debian/rules (+1/-1)
storage/innobase/xtrabackup/utils/percona-xtrabackup.spec (+7/-1)
To merge this branch: bzr merge lp://qastaging/~tplavcic/percona-xtrabackup/bug1156209-2.2
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Review via email: mp+247329@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2015-01-21.

Description of the change

Man pages were added before but they were not build and added into packages/tarball.
Changes were made to our build nodes to support sphinx-build and also made changes in cmake and packages to generate man pages on build and include them into packages.
Removed: Makefile and make.bat from utils/doc directory.
Added -DWITH_MAN_PAGES option which is turned on by default.
Added -DWITH_HTML_DOCS, -DWITH_LATEX_DOCS, -DWITH_PDF_DOCS options which are turned off by default.

To post a comment you must log in.
Revision history for this message
Tomislav Plavcic (tplavcic) wrote : Posted in a previous version of this proposal

Test packages are available here:
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.2-RELEASE/46/
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.3-RELEASE/4/
nevermind the yellow build for 2.3 for centos5-32 it was some connection problem and it was successful when restarted.

Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

General comments:

- shouldn't we remove doc/Makefile and doc/make.bat? That would also require re-imlementing the remaining bits from Makefile in doc/source/CMakeLists.txt. How much work would that require?

- please add a CMake configuration option (-DWITH_MAN_PAGES, ON by default) so that it would be possible to build xtrabackup without man pages on systems without sphinx installed.

review: Needs Fixing
Revision history for this message
Tomislav Plavcic (tplavcic) wrote : Posted in a previous version of this proposal

Thanks Alexey, some good comments.
1. Will check with Hrvoje which building options he actually uses and what it would take to move them.
2. Will do.

Revision history for this message
Tomislav Plavcic (tplavcic) wrote : Posted in a previous version of this proposal
Download full text (4.2 KiB)

Resubmitting MP - here are the changes:
Regarding #1:
I have removed Makefile and make.bat from doc and added into cmake builds for html, latex and pdf - checked with Hrvoje and it seems that are the ones we use. Also changed jenkins documentation jobs to reflect new changes.

Regarding #2: Added option -DWITH_MAN_PAGES which is turned on by default. Also new options which are turned off by default are -DWITH_HTML_DOCS, -DWITH_LATEX_DOCS and -DWITH_PDF_DOCS and those are currently used by new jenkins documentation jobs.

Here's some testing:
PXB 2.2:
--------
TEST PACKAGES:
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.2-RELEASE/47/

RPM:
plavi@zoidberg:pxb $ rpm -qlp percona-xtrabackup-2.2.7-5061.el6.x86_64.rpm|grep man
/usr/share/man/man1/innobackupex.1.gz
/usr/share/man/man1/xbcrypt.1.gz
/usr/share/man/man1/xbstream.1.gz
/usr/share/man/man1/xtrabackup.1.gz

DEB:
plavi@zoidberg:pxb $ dpkg -c percona-xtrabackup_2.2.7-5061-1.trusty_amd64.deb|grep man
drwxr-xr-x root/root 0 2015-01-20 17:11 ./usr/share/man/
drwxr-xr-x root/root 0 2015-01-20 17:11 ./usr/share/man/man1/
-rw-r--r-- root/root 24486 2015-01-20 17:11 ./usr/share/man/man1/innobackupex.1.gz
-rw-r--r-- root/root 808 2015-01-20 17:11 ./usr/share/man/man1/xbcrypt.1.gz
-rw-r--r-- root/root 20415 2015-01-20 17:11 ./usr/share/man/man1/xtrabackup.1.gz
-rw-r--r-- root/root 1230 2015-01-20 17:11 ./usr/share/man/man1/xbstream.1.gz

TAR:
plavi@zoidberg:pxb $ tar -ztvf percona-xtrabackup-2.2.7-5061-Linux-x86_64.tar.gz|grep man
drwxr-xr-x root/root 0 2015-01-20 17:09 percona-xtrabackup-2.2.7-Linux-x86_64/man/
drwxr-xr-x root/root 0 2015-01-20 17:09 percona-xtrabackup-2.2.7-Linux-x86_64/man/man1/
-rw-r--r-- root/root 69420 2015-01-20 17:07 percona-xtrabackup-2.2.7-Linux-x86_64/man/man1/xtrabackup.1
-rw-r--r-- root/root 2084 2015-01-20 17:07 percona-xtrabackup-2.2.7-Linux-x86_64/man/man1/xbcrypt.1
-rw-r--r-- root/root 92762 2015-01-20 17:07 percona-xtrabackup-2.2.7-Linux-x86_64/man/man1/innobackupex.1
-rw-r--r-- root/root 2834 2015-01-20 17:07 percona-xtrabackup-2.2.7-Linux-x86_64/man/man1/xbstream.1

JENKINS DOCUMENTATION JOBS:
http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-documentation-pdf-new/3/
http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-documentation-new/1/

PXB 2.3:
--------
TEST PACKAGES:
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.3-RELEASE/6/

RPM:
plavi@zoidberg:pxb $ rpm -qlp percona-xtrabackup-23-2.3.0-5062.alpha1.el7.x86_64.rpm |grep man
/usr/share/man/man1/innobackupex.1.gz
/usr/share/man/man1/xbcrypt.1.gz
/usr/share/man/man1/xbstream.1.gz
/usr/share/man/man1/xtrabackup.1.gz

DEB:
plavi@zoidberg:pxb $ dpkg -c percona-xtrabackup-23_2.3.0-5062.alpha1-1.trusty_amd64.deb|grep man
drwxr-xr-x root/root 0 2015-01-21 09:23 ./usr/share/man/
drwxr-xr-x root/root 0 2015-01-21 09:23 ./usr/share/man/man1/
-rw-r--r-- root/root 24487 2015-01-21 09:23 ./usr/share/man/man1/innobackupex.1.gz
-rw-r--r-- root/root 811 2015-01-21 09:23 ./usr/share/man/man1/xbcrypt.1.gz
-rw-r--r-- root/root 20415 2015-01-...

Read more...

Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

- the year in the copyright notice should be 2015, not 2013 (I mentioned it in the comments for the previous MP)

- storage/innobase/xtrabackup/doc/build/ should be removed from .bzrignore (since we no longer create that diretory) and replaced with storage/innobase/xtrabackup/doc/source/build/

- I don't think using relative path in CMakeLists.txt is a good idea. One may want to do out-of-source builds, for example.

- I have configured the source tree as follows:

cmake -DWITH_HTML_DOCS=on -DWITH_LATEX_DOCS=on -DWITH_PDF_DOCS=on -DWITH_MAN_PAGES=on .

Then 'make' failed for me like this:

[ 0%] Generating build/latex/PerconaXtraBackup-2.2.pdf
make[3]: *** No rule to make target `all-pdf'. Stop.
make[2]: *** [storage/innobase/xtrabackup/doc/source/build/latex/PerconaXtraBackup-2.2.pdf] Error 2
make[1]: *** [storage/innobase/xtrabackup/doc/source/CMakeFiles/latexpdf.dir/all] Error 2
make: *** [all] Error 2

What am I missing?

review: Needs Fixing
Revision history for this message
Tomislav Plavcic (tplavcic) wrote : Posted in a previous version of this proposal

Fixed:
- year in copyright
- doc/build in .bzrignore
- added relative paths in CMakeLists.txt

I have tried to do out-of-source builds and in-source builds and it worked for me - also tried to do build with all options like you did and it worked (probably didn't work before because of relative path or something).

There is some bug in sphinx regarding the html_theme_path option which has been resolved just recently and because of this I have left that the percona-theme is downloaded in source dir and removed after build - that only makes difference if you do out-of-source build of html docs.
I have left the comment in CMakeLists for this, and the bug is this one:
https://github.com/sphinx-doc/sphinx/issues/925

Test builds:
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.2-RELEASE/48/
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.3-RELEASE/7/

Revision history for this message
Alexey Kopytov (akopytov) wrote :

I still can reproduce it as follows:

$ bzr branch lp:~tplavcic/percona-xtrabackup/bug1156209-2.2
$ cd bug1156209-2.2
$ cmake -DWITH_HTML_DOCS=on -DWITH_LATEX_DOCS=on -DWITH_PDF_DOCS=on -DWITH_MAN_PAGES=on .
$ make -j3

If I do "cd storage/innobase/xtrabackup/doc/source; make -j3" after the above fails, everything builds successfully.

review: Needs Fixing
Revision history for this message
Tomislav Plavcic (tplavcic) wrote :

Yep, you're right.
Seems the problem was in badly defined outputs and dependencies for latexpdf target since it depends on the output of latex target - it needs Makefile which is produced there and tex file also - when parallel building was done it only looked for existance of doc/source/build/latex directory but final and needed outputs maybe were not created yet - at least that is my current understanding.

So I managed to reproduce your error and with this new commit I cannot reproduce it so please check.

Here's jenkins jobs with some testing:
TEST:
pdf job
http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-documentation-pdf-new/5/
html job:
http://jenkins.percona.com/view/PXB%202.2/job/percona-xtrabackup-2.2-documentation-new/4/
release packages job:
http://jenkins.percona.com/view/Percona-RELEASES/job/percona-xtrabackup-2.2-RELEASE/50/

Revision history for this message
Alexey Kopytov (akopytov) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches