Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-5802

Cmake in mariadbclient finds wrong libz

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Story Points:
      1
    • Team:
      Architecture

      Description

      When building mariadbclient, cmake identifies libz from a separate python installation than the one setup to run the stack. I have an anaconda installation on the disk, and a miniconda installation set up specifically for the lsst stack. During the building process CMake for some reason finds the alternate libz associated with that python installation.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            I think we have to bite the bullet and just stop mariadb from looking for an external libz and use the bundled version. I would guess this problem can hit on Linux as well.

            ARGS+=('-DWITH_ZLIB=bundled')
            

            An alternative is to sanitize the PATH before calling cmake.

            Show
            tjenness Tim Jenness added a comment - I think we have to bite the bullet and just stop mariadb from looking for an external libz and use the bundled version. I would guess this problem can hit on Linux as well. ARGS+=('-DWITH_ZLIB=bundled') An alternative is to sanitize the PATH before calling cmake .
            Hide
            tjenness Tim Jenness added a comment -

            John would you mind taking a look at this? It passes Jenkins and is fine for me on OS X. I agree that this approach may well tickle an edge case but it does seem a bit more robust than hoping that there is only one python in the path. I'll add that when doing conda builds you end up with `libz` in two locations because the `python` comes from a different location to `conda` and they both have `libz` in the tree. Fiddling with PATH fixes that anomaly as well (As would bundling libz).

            I'm happy to switch to the libz bundle on both Linux and OS X if that would make you feel happier but then we are still left with the Linux openssl problem described in DM-5170.

            Show
            tjenness Tim Jenness added a comment - John would you mind taking a look at this? It passes Jenkins and is fine for me on OS X. I agree that this approach may well tickle an edge case but it does seem a bit more robust than hoping that there is only one python in the path. I'll add that when doing conda builds you end up with `libz` in two locations because the `python` comes from a different location to `conda` and they both have `libz` in the tree. Fiddling with PATH fixes that anomaly as well (As would bundling libz). I'm happy to switch to the libz bundle on both Linux and OS X if that would make you feel happier but then we are still left with the Linux openssl problem described in DM-5170 .
            Hide
            swinbank John Swinbank added a comment -

            Hmm.

            I don't much like this, but I agree that it's probably a more robust solution than the previous one. It still seems worryingly fragile though: I'm worried we'll hit edge cases, and, while I know nothing about conda-build, the comments on the commit imply that Josh has similar concerns.

            Assuming we don't just want to use the bundled libraries (and that would also make me a bit uncomfortable, but not enough to make me seriously object), I wonder if a thorough solution would actually involve patching the package so that the CMake code uses NO_SYSTEM_ENVIRONMENT_PATH internally, hence avoiding any need to futz with the user's path. Seems like that might involve us carrying a non-trivial patch in our package, though, which is also ugly.

            In short... I'm not confident that this is a good, long-term, robust solution. But I have no obviously better ideas, so I'm happy for you to go ahead and merge.

            Show
            swinbank John Swinbank added a comment - Hmm. I don't much like this, but I agree that it's probably a more robust solution than the previous one. It still seems worryingly fragile though: I'm worried we'll hit edge cases, and, while I know nothing about conda-build , the comments on the commit imply that Josh has similar concerns. Assuming we don't just want to use the bundled libraries (and that would also make me a bit uncomfortable, but not enough to make me seriously object), I wonder if a thorough solution would actually involve patching the package so that the CMake code uses NO_SYSTEM_ENVIRONMENT_PATH internally, hence avoiding any need to futz with the user's path. Seems like that might involve us carrying a non-trivial patch in our package, though, which is also ugly. In short... I'm not confident that this is a good, long-term, robust solution. But I have no obviously better ideas, so I'm happy for you to go ahead and merge.
            Hide
            tjenness Tim Jenness added a comment -

            Yes. I share your concern and Joshua Hoblitt's but it does seem less flaky than the previous scheme (which has proven itself to not be robust). It would be nice if we could get conda build tested rather than just assume it's going to be a problem. The real issue will be if gcc is in the same place as the libraries we are trying to avoid (I don't know how a conda environment picks up gcc4.8+).

            Show
            tjenness Tim Jenness added a comment - Yes. I share your concern and Joshua Hoblitt 's but it does seem less flaky than the previous scheme (which has proven itself to not be robust). It would be nice if we could get conda build tested rather than just assume it's going to be a problem. The real issue will be if gcc is in the same place as the libraries we are trying to avoid (I don't know how a conda environment picks up gcc4.8+).
            Hide
            tjenness Tim Jenness added a comment -

            As discussed on the community post, Mario Juric requests that we use bundled libz and ssl and I've adjusted the patch to implement that.

            Show
            tjenness Tim Jenness added a comment - As discussed on the community post , Mario Juric requests that we use bundled libz and ssl and I've adjusted the patch to implement that.
            Hide
            FabioHernandez Fabio Hernandez added a comment -

            Is there any chance that the mariadbclient built this way be transportable among Linux distributions? In particular, if I build the stack on CentOS will I be able to use it on Ubuntu? I'm interested in this use case for binary distribution of the stack, so that I can have a self-contained single binary stack executable on several Linux distributions.

            Show
            FabioHernandez Fabio Hernandez added a comment - Is there any chance that the mariadbclient built this way be transportable among Linux distributions? In particular, if I build the stack on CentOS will I be able to use it on Ubuntu? I'm interested in this use case for binary distribution of the stack, so that I can have a self-contained single binary stack executable on several Linux distributions.
            Hide
            swinbank John Swinbank added a comment - - edited

            I share Josh's concern: not a fan of this (revised) solution.

            Show
            swinbank John Swinbank added a comment - - edited I share Josh's concern : not a fan of this (revised) solution.
            Hide
            tjenness Tim Jenness added a comment -

            Fabio Hernandez I think it will be more portable built this way.

            John Swinbank yes, but it depends a bit on whether we are actually using the SSL interface. Fritz Mueller?

            Show
            tjenness Tim Jenness added a comment - Fabio Hernandez I think it will be more portable built this way. John Swinbank yes, but it depends a bit on whether we are actually using the SSL interface. Fritz Mueller ?
            Hide
            ktl Kian-Tat Lim added a comment -

            Binary distributions of Enterprise MySQL apparently use the "bundled" yaSSL rather than OpenSSL. I think that means that it is safe enough for us to use it as well, at least for the time being. The risks and potential impact of an SSL-layer security breach for MySQL access are pretty low at this point, noting that passwords are not secured by SSL.

            In the longer run, for better security, it would be good to find a way to use a common OpenSSL for all parts of the stack. But in the short run, I'm OK with moving forward with bundled yaSSL for mariadbclient.

            Show
            ktl Kian-Tat Lim added a comment - Binary distributions of Enterprise MySQL apparently use the "bundled" yaSSL rather than OpenSSL. I think that means that it is safe enough for us to use it as well, at least for the time being. The risks and potential impact of an SSL-layer security breach for MySQL access are pretty low at this point, noting that passwords are not secured by SSL. In the longer run, for better security, it would be good to find a way to use a common OpenSSL for all parts of the stack. But in the short run, I'm OK with moving forward with bundled yaSSL for mariadbclient .
            Hide
            tjenness Tim Jenness added a comment -

            I take this as approval that I should merge the mariadbclient changes and leave mariadb unchanged. The latter will only cause issues if mariadb is being used as a server on a machine that differs to the one it was built on.

            Show
            tjenness Tim Jenness added a comment - I take this as approval that I should merge the mariadbclient changes and leave mariadb unchanged. The latter will only cause issues if mariadb is being used as a server on a machine that differs to the one it was built on.
            Hide
            mjuric Mario Juric added a comment -

            > As discussed on the community post, Mario Juric requests that we use bundled libz and ssl and I've adjusted the patch to implement that.

            Just to clarify the process (as I've seen the same misunderstanding arise elsewhere): this wasn't meant to be a formal request, just a question/comment! What goes into a release (including potential workarounds) is within the release manager's scope of authority, with some oversight from Architecture re long-term concerns (e.g. cybersecurity).

            Show
            mjuric Mario Juric added a comment - > As discussed on the community post, Mario Juric requests that we use bundled libz and ssl and I've adjusted the patch to implement that. Just to clarify the process (as I've seen the same misunderstanding arise elsewhere): this wasn't meant to be a formal request, just a question/comment! What goes into a release (including potential workarounds) is within the release manager's scope of authority, with some oversight from Architecture re long-term concerns (e.g. cybersecurity).
            Hide
            tjenness Tim Jenness added a comment -

            Mario Juric your comments carry weight on this project

            My job is to get everyone to (finally) agree on a fix (this ticket was filed two months ago) so I'm glad we have some form of resolution. What goes in the release is a completely different issue. I'm just happy to be able to close the ticket.

            Show
            tjenness Tim Jenness added a comment - Mario Juric your comments carry weight on this project My job is to get everyone to (finally) agree on a fix (this ticket was filed two months ago) so I'm glad we have some form of resolution. What goes in the release is a completely different issue. I'm just happy to be able to close the ticket.
            Hide
            tjenness Tim Jenness added a comment -

            I have merged the mariadbclient fix and left mariadb using the system libraries. It's trivial to also switch mariadb to bundled libraries if we need to do that later on.

            Show
            tjenness Tim Jenness added a comment - I have merged the mariadbclient fix and left mariadb using the system libraries. It's trivial to also switch mariadb to bundled libraries if we need to do that later on.

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              nlust Nate Lust
              Reviewers:
              John Swinbank, Joshua Hoblitt
              Watchers:
              Fabio Hernandez, Frossie Economou, John Swinbank, Joshua Hoblitt, Kian-Tat Lim, Mario Juric, Nate Lust, Tim Jenness, Yvan Calas
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.