install-deps.sh: use mk-build-deps instead of processing control - #8286
install-deps.sh: use mk-build-deps instead of processing control#8286dmick wants to merge 1 commit into
Conversation
|
probably want to go together with #8285 |
|
it looks good to me but requires careful testing in all the contexts in which install-deps.sh is used |
|
I tested on a jessie VM, primarily, but also verified on trusty desktop (both running script by hand). |
|
it would be good to push that to the gitbuilders since they use install-deps.sh as well |
Fixes: #8286 Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
ghost
reopened this
Since the "Fixes: #..." line is interpreted by GitHub as referring to a pull request, yet the intention is for it to refer to the tracker issue, change our instructions to use the full tracker issue URL. Now that github pull request numbers are in the 8000s, there are cases when a PR is wrongly closed by a "Fixes: #..." line in a commit message. See ceph#8286 (comment) for one such case. Signed-off-by: Nathan Cutler <ncutler@suse.com>
|
The gitbuilders should use the copy from the ceph repo, no?.... |
|
@dmick I don't remember how the gitbuilder obtain install-deps.sh but it would make sense if they used the one found in the branch to be built |
|
Pretty sure they do. Anything else we need for this PR? |
|
@dachary ping |
|
@dmick is there a need to merge this in jewel ? If not and since it's so easy to get this slightly wrong and have it bug us a few times, could we wait until jewel is cut ? If there is a motive it looks good to merge, just being cautious :-) |
|
I don't guess I care much. In my mind the bigger release risk now is the jenkins builds, and they're not affected. |
|
please test this |
|
test this please |
|
ping |
|
@dachary do we need to do anything else with this? |
|
test this please |
ghost
removed their assignment
|
I don't want to stall this PR, someone else can take over. |
|
test this please |
|
You are missing the equivs package. |
|
@ErwanAliasr1: I'm confused. The changes include installing devscripts, which depends on equivs. Did you see that error message somewhere? Answer: yes, in the failed check build. I'm still confused. |
|
@dmick I saw that message from the build spawn by your PR. |
|
retest this please |
|
Hm, well it's gonna pass this time because it's not running the test on an apt machine :( |
88ebfcc to
636622d
Compare
|
So it still has this problem when it runs on a trusty machine, and I don't know why. equivs is a dependency of devscripts on trusty; it should be installed. O_o |
636622d to
db8c31e
Compare
|
equivs was a "Suggests:", even though it shows up after installation as a "Depends:". Whatever. Added it to be explicitly installed, and that made it work. |
db8c31e to
c1f381c
Compare
There was a problem hiding this comment.
maybe use --remove here too? not sure it matters really, since it'll overwrite an existing file just fine
There was a problem hiding this comment.
the comment indicates I once had at least contemplated --remove. I wonder why I removed it.
|
pushed to wip-dmick-install-deps (on top of master) to try this out on the gitbuilders |
mk-build-deps is designed to handle installing build dependencies; use that instead, so '|' indications are handled properly. Signed-off-by: Dan Mick <dan.mick@redhat.com>
c1f381c to
bdcd6f1
Compare
|
Looks good to me |
|
@jdurgin: anything else you think we should do? |
|
@dmick looks good to me, merged manually to master cherry-picked to jewel |
3 participants
mk-build-deps is designed to handle installing build dependencies;
use that instead, so '|' indications are handled properly.
Signed-off-by: Dan Mick dan.mick@redhat.com