[Maria-developers] Extra copying in debian/rules causes troubles
Hi all. There is a fragment in MariaDB debian/dist/*/rules files: ... ifeq ($(findstring nocheck,$(DEB_BUILD_OPTIONS)),) # Don't know why the following is necessary... cp unittest/unit.pl $(builddir)/unittest/ cp -r mysql-test/* $(builddir)/mysql-test/ cp -r sql/share/* $(builddir)/sql/share/ cp -r scripts/*sql $(builddir)/scripts/ if [ ! -f testsuite-stamp ] ; then \ ... Recently it started causing a problem. MTR assumes that the tests are run either in a proper source build (be it in-source or out-of-source), or in a binary build. With the extra-copying above, particularly due to copying of mysql-test folder, the builddir by the moment of running tests is in the "neither fish nor fowl" state. So if one runs dpkg-buildpackage with the tests enabled, it fails when it comes to the MTR stage. We had this problem reported at least 3 times over the last 24 hours, so it's not hypothetical. It's currently filed as https://mariadb.atlassian.net/browse/MDEV-5591. I tried to remove these 4 'cp' lines and the build seemed to work all right; but there might also be consequences that I fail to predict, so I would like to have the 2nd (3rd, 4th, ... Nth) opinion. Does anyone have any idea what might go wrong without these 4 lines? Also, *Kristian*, These lines are coming from the initial draft of the rules. I understand from the comment that there is no reason to ask *why* the lines were necessary, but I'm wondering if you remember why you felt they *were* necessary (even if didn't know why). Were you getting some errors without them? Or were they copied from some previous version of the file? I realize it was long time ago so you probably won't remember, but I thought it was worth checking anyway. Regards, Elena
Elena Stepanova <elenst@montyprogram.com> writes:
ifeq ($(findstring nocheck,$(DEB_BUILD_OPTIONS)),) # Don't know why the following is necessary... cp unittest/unit.pl $(builddir)/unittest/ cp -r mysql-test/* $(builddir)/mysql-test/ cp -r sql/share/* $(builddir)/sql/share/ cp -r scripts/*sql $(builddir)/scripts/ if [ ! -f testsuite-stamp ] ; then \
Recently it started causing a problem. MTR assumes that the tests are run either in a proper source build (be it in-source or out-of-source), or in a binary build. With the extra-copying above, particularly due to copying of mysql-test folder, the builddir by the moment of running tests is in the "neither fish nor fowl" state.
These lines are coming from the initial draft of the rules. I understand from the comment that there is no reason to ask *why* the lines were necessary, but I'm wondering if you remember why you felt they *were* necessary (even if didn't know why). Were you getting some errors without them? Or were they copied from some previous version of the file?
I am pretty sure they were copied from previous version, possibly even also similarly copied by OurDelta from original ancient Debian packaging. Maybe they were used to be able to run mysql-test inside the build directory in the autotools days? Note that in MariaDB, we never ran mysql-test that way, instead we install the mariadb-test-X.Y package and run the tests there (it always seemed a bit silly to me to test the build, not the packages - what is the point of a good build, if it is packaged incorrectly?)
I tried to remove these 4 'cp' lines and the build seemed to work all right; but there might also be consequences that I fail to predict, so I would like to have the 2nd (3rd, 4th, ... Nth) opinion. Does anyone have any idea what might go wrong without these 4 lines?
Well, I can think of two things: 1. It could affect running mysql-test inside the build directory. Well, in fact it does, since you found that it fixes this :-) 2. It could affect what gets installed by `make install` into the mariadb-test-X.Y package. I suggest you check the contents of that package with and without those lines. If the contents is the same, just go ahead and remove it. If anything is missing, probably the lines should be removed anyway, and cmake should be fixed to install things properly. In short, removing sounds fine, it's probably just some left-over crap like so much else of the .deb packaging. - Kristian.
Hi, On 1/31/2014 11:49 AM, Kristian Nielsen wrote:
Elena Stepanova <elenst@montyprogram.com> writes:
ifeq ($(findstring nocheck,$(DEB_BUILD_OPTIONS)),) # Don't know why the following is necessary... cp unittest/unit.pl $(builddir)/unittest/ cp -r mysql-test/* $(builddir)/mysql-test/ cp -r sql/share/* $(builddir)/sql/share/ cp -r scripts/*sql $(builddir)/scripts/ if [ ! -f testsuite-stamp ] ; then \
<cut>
I tried to remove these 4 'cp' lines and the build seemed to work all right; but there might also be consequences that I fail to predict, so I would like to have the 2nd (3rd, 4th, ... Nth) opinion. Does anyone have any idea what might go wrong without these 4 lines?
Well, I can think of two things:
1. It could affect running mysql-test inside the build directory. Well, in fact it does, since you found that it fixes this :-)
2. It could affect what gets installed by `make install` into the mariadb-test-X.Y package. I suggest you check the contents of that package with and without those lines.
I did the check and have not found any differences in package contents. The only thing I did differently from "normal" autobake-deb.sh was excluding oqgraph since it does not build on my machine; but I suppose these changes should not affect it anyway.
If the contents is the same, just go ahead and remove it.
I'm ready to do so, but Otto says that he's currently working on the contents of the debian folder and is getting ready to push it, and thus I should probably not change anything in there. On the other hand, it might be nice to have the change in 10.0.8, to let it run the tests after package build again. All in all, I need to know how to proceed -- push or not push. Regards, /E
If anything is missing, probably the lines should be removed anyway, and cmake should be fixed to install things properly.
In short, removing sounds fine, it's probably just some left-over crap like so much else of the .deb packaging.
- Kristian.
Elena Stepanova <elenst@montyprogram.com> writes:
I'm ready to do so, but Otto says that he's currently working on the contents of the debian folder and is getting ready to push it, and thus I should probably not change anything in there. On the other hand, it might be nice to have the change in 10.0.8, to let it run the tests after package build again. All in all, I need to know how to proceed -- push or not push.
From my point of view the change is fine to push. But I agree it sounds a good idea to coordinate with Otto.
- Kristian.
2014-02-04 Kristian Nielsen <knielsen@knielsen-hq.org>:
Elena Stepanova <elenst@montyprogram.com> writes:
I'm ready to do so, but Otto says that he's currently working on the contents of the debian folder and is getting ready to push it, and thus I should probably not change anything in there. On the other hand, it might be nice to have the change in 10.0.8, to let it run the tests after package build again. All in all, I need to know how to proceed -- push or not push.
From my point of view the change is fine to push. But I agree it sounds a good idea to coordinate with Otto.
If this change actually fixes the bug or not I will comment to https://mariadb.atlassian.net/browse/MDEV-5591 once my tests are done I think you can push to can push on mariadb-10 freely. I have diverged only mariadb-5.5/debian/* in my https://github.com/ottok/mariadb-5.5. The world will not collapse if you push in 5.5 too, it's just that life will be a bit simpler if my mariadb-5.5/debian/* will eventually be merged upon the original mariadb-5.5/debian/* and not need a 3-way merge.
participants (3)
-
Elena Stepanova -
Kristian Nielsen -
Otto Kekäläinen