MDEV-28153: Debian autobake- use absolute dependencies #2064
Conversation
|
Changed to |
Since MDEV-28153, debian dependencies are now a property of the script. Without adjusting control before
By running the prepare only of autobake-deb.sh, the control file has its build dependencies updated. Running
A flag to run the prepare stage only, so that
It need to be id-potent so that a second run to trigger the build doesn't get two sets of included columnstore files, especially in |
There is no need to run prepare only, either you run full autobake-deb.sh or you do a native build. A change based on the above assumption is mislead.
This command works all the time and is part of standard Debian base. Why add it now? This PR would be a lot more convincing if it referred a failing CI run on mainline and then a successful one from this PR. That is not the case now.
Does not make sense as explained above.
This whole section is obsolete now when you changed/broke the previous columnstore packaging structure via ec62f46. Either it is part of the native packaging (withfiles in debian/* and stanza in debian/control), or it is not. Any attempt for middle ground just causes a mess and is brings no benefit as far as I can see. |
https://salsa.debian.org/grooverdan/mariadb-server/-/jobs/2603984
I noticed and corrected this 2885fb0 I'm sorry we have differing opinions on the MDEV-28153 changes. |
|
Thanks for 2885fb0. Please in the future ask for review for all commits before you push them. The PR where you pushed that commit you added those commits after review and proceeded to merge without waiting for second review of the updated version. I see in https://salsa.debian.org/grooverdan/mariadb-server/-/jobs/2603984 To iterate, Salsa-CI builds on branches 10.5/6/7/8/9 are not broken and do not need this fix.
I still think that the claim about "apt cache being broken" is based on misunderstanding or flawed assumptions. The apt cache is never broken if the builder is healthy and capable of installing new packages. However, since you now refactored that, you should take ownership of the autobake-deb.sh contents. You can do that without having to pollute the salsa-ci.yml file with (at least so far seemingly) unnecessary |
debian/autobake-deb.sh
Outdated
| @@ -91,6 +94,11 @@ case "${CODENAME}" in | |||
| exit 1 | |||
| esac | |||
|
|
|||
| if [ -n "${SALSA_PREP:-}" ] | |||
There was a problem hiding this comment.
There could be some comment why this is here as I don't understood reasoning? If we are not in Salsa exit?
There was a problem hiding this comment.
:- to give it a default value in case its not set. Will prevent failures in bash strict +x mode and gets recognised by shellcheck.
There was a problem hiding this comment.
Please add inline comment on what this section is, what uses it and why. Anybody can look up the syntax on a man page but it is hard to guess why you have this seemingly unnecessary logic here.
There was a problem hiding this comment.
Ok, so there should be an inline comment saying that the section is tailored for use with https://github.com/MariaDB/mariadb.org-tools/blob/master/buildbot.mariadb.org/ci_build_images/debian.Dockerfile to make Docker images with preinstalled build-dependencies for each Debian/Ubuntu release, and thus you want to generate the debian/control files but not run autobake.
grooverdan
force-pushed
the
bb-10.5-danielblack-MDEV-28153-deb-explict-deps-salsa-postfix
branch
from
238a7a6
to
35cb5b4
Compare
grooverdan
changed the title
|
Removed salsa line, Added the one dependency line only. https://salsa.debian.org/grooverdan/mariadb-server/-/commit/35cb5b446cbbffa6729e5d55d81c224ed56d06d9/pipelines?ref=bb-10.5-danielblack-MDEV-28153-deb-explict-deps-salsa-postfix is the build in progress. Matched up with MariaDB/mariadb.org-tools#130 build. |
grooverdan
force-pushed
the
bb-10.5-danielblack-MDEV-28153-deb-explict-deps-salsa-postfix
branch
from
35cb5b4
to
7fbc52c
Compare
Without doing the full build. Autobake now includes a dependency on lsb-release. As the BB CI images (https://github.com/MariaDB/mariadb.org-tools/blob/master/buildbot.mariadb.org/ci_build_images/debian.Dockerfile) have explicit dependencies, there's no point maintaining them in two places. We don't want do the full autobake-deb.sh there, just enough to have the control file containing the correct dependencies. Helps: MariaDB/mariadb.org-tools#130
Rather than Debian logs containing a barely decipherable mix of build command and the output of some other command, we use the make option --output-sync=target. This make the compile line and the output stay together in the output stream. This option exists even in the make version in debian;stretch so should work everywhere. Test on debian:stretch, ubuntu:18.04, the two lowest version that use the debian/rules.
grooverdan
force-pushed
the
bb-10.5-danielblack-MDEV-28153-deb-explict-deps-salsa-postfix
branch
from
7fbc52c
to
81418f4
Compare
grooverdan
deleted the
bb-10.5-danielblack-MDEV-28153-deb-explict-deps-salsa-postfix
branch
Yes, but I am asking where is the build that failed? The addition you did to salsa-ci.yml is unnecessary, it does not do anything. Referencing a build that passes does not prove anything about that that addition fixed (and it didn't fix anything, salsa-ci on 10.5 is already passing). |
Why does it not do anything? Is lsb already Salsa-CI dependency? I think point is that it's not breaking anything it's not that it should make it pass? |
Description
With autobake-deb.sh no doing explicit dependencies, we need to run this
explicitly as a prep step in salsa-ci so the dependencies can be pulled
before the real compilation begins.
Resolves 10.5 part of salsa in #2039.
How can this PR be tested?
Currently running on https://salsa.debian.org/grooverdan/mariadb-server/-/pipelines/361865)
(10.8 test timed out)
Basing the PR against the correct MariaDB version
Backward compatibility
TODO: fill details here, if applicable, or remove the section