Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mysql80: init at 8.0.17 #65221

Merged
merged 5 commits into from Aug 13, 2019
Merged

mysql80: init at 8.0.17 #65221

merged 5 commits into from Aug 13, 2019

Conversation

totten
Copy link
Contributor

@totten totten commented Jul 21, 2019

Motivation for this change

MySQL 8.0 is a new shiny. Need mysql80 binaries so that we can start testing our app with it.

Related links:

The PR introduces version 8.0.16 8.0.17.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Other comments

This is my first submission to nixpkgs. It's just an adaptation of the mysql57 scripts, with updates for compiler compatibility.

I've done very light testing (manually launching mysqld; using mysql to create a small database+table).

MySQL 8.0 is a significant iteration after MySQL 5.7.  This patch adds it as
a parallel build alongside mysql57 (similar to mysql56 and mysql55 before).
@aanderse
Copy link
Member

aanderse commented Aug 4, 2019

@totten Thanks for deciding to contribute to nixpkgs! It looks like 8.0.17 has been released so when you get a chance would you mind rewriting this PR to use the latest version please?

ping @Izorkin in case you're interested.

@aanderse aanderse requested a review from fpletz August 4, 2019 11:51
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few minor things I'll point out. I only did a very minor 'cosmetic' review of the package. I'm going to leave real review for the big hitters who are familiar with building mysql.

pkgs/servers/sql/mysql/8.0.x.nix Outdated Show resolved Hide resolved
pkgs/servers/sql/mysql/8.0.x.nix Outdated Show resolved Hide resolved
pkgs/servers/sql/mysql/8.0.x.nix Outdated Show resolved Hide resolved
pkgs/servers/sql/mysql/8.0.x.nix Outdated Show resolved Hide resolved
Note: "MySQL Router" is a distinct component. Building it fails on macOS 10.12.6 with an error (below); hence, it is skipped.

```
[ 98%] Building CXX object router/src/plugin_info/src/CMakeFiles/mysqlrouter_plugin_info.dir/library_file.cc.o
[ 98%] Building CXX object router/src/plugin_info/src/CMakeFiles/mysqlrouter_plugin_info.dir/main.cc.o
[ 98%] Building CXX object router/src/plugin_info/src/CMakeFiles/mysqlrouter_plugin_info.dir/plugin_info_app.cc.o
[ 98%] Building CXX object router/src/plugin_info/src/CMakeFiles/mysqlrouter_plugin_info.dir/plugin.cc.o
[ 98%] Linking CXX executable ../../../../runtime_output_directory/mysqlrouter_plugin_info
[ 98%] Built target mysqlrouter_plugin_info
Scanning dependencies of target rest_api_lib
[ 98%] Building CXX object router/src/rest_api/src/CMakeFiles/rest_api_lib.dir/rest_api_utils.cc.o
[ 98%] Linking CXX static library librest_api_lib.a
[ 98%] Built target rest_api_lib
Scanning dependencies of target rest_api
[ 98%] Building CXX object router/src/rest_api/src/CMakeFiles/rest_api.dir/rest_api_component.cc.o
In file included from /tmp/nix-build-mysql-8.0.17.drv-0/mysql-8.0.17/router/src/rest_api/src/rest_api_component.cc:30:
/tmp/nix-build-mysql-8.0.17.drv-0/mysql-8.0.17/router/src/rest_api/src/rest_api_plugin.h:90:8: error: 'shared_timed_mutex' is unavailable: introduced in macOS 10.12
  std::shared_timed_mutex rest_api_handler_mutex_;
       ^
/nix/store/3r156gjbpig7xav3crnhffn3msnqmak7-libc++-7.1.0/include/c++/v1/shared_mutex:204:58: note: 'shared_timed_mutex' has been explicitly marked unavailable here
class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_timed_mutex
                                                         ^
1 error generated.
make[2]: *** [router/src/rest_api/src/CMakeFiles/rest_api.dir/build.make:63: router/src/rest_api/src/CMakeFiles/rest_api.dir/rest_api_component.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:10908: router/src/rest_api/src/CMakeFiles/rest_api.dir/all] Error 2
make: *** [Makefile:152: all] Error 2
builder for '/nix/store/19c1b8qn0x8fnsa64y4k19r169ifv1b7-mysql-8.0.17.drv' failed with exit code 2
error: build of '/nix/store/19c1b8qn0x8fnsa64y4k19r169ifv1b7-mysql-8.0.17.drv' failed
```

Would skipping it be a regression?  I don't think so: `mysqlrouter` was not
part of `mysql57`, and `mysql80` has not been previously published via
nixpkgs.
@totten
Copy link
Contributor Author

totten commented Aug 6, 2019

Thanks for deciding to contribute to nixpkgs! It looks like 8.0.17 has been released so when you get a chance would you mind rewriting this PR to use the latest version please?

Alrighty, I've pushed up 585da78 to use 8.0.17.

There was one snag with 8.0.17 -- the mysql8 source tree includes a bundled copy of mysqlrouter, but this isn't building with Darwin and 8.0.17. (See commit message for details.) However, it's basically a new feature and distinct submodule (which wasn't in mysql57, mysql56, etc), so I suppose there's no major harm in skipping it for the moment?

@totten totten changed the title mysql80: init 8.0.16 mysql80: init 8.0.17 Aug 7, 2019
@orivej
Copy link
Contributor

orivej commented Aug 11, 2019

I have cleaned it up and consider it ready to merge (as a single squashed commit).

Some notes:

  1. 8.0-gc-sections.patch is not needed because mysql checks if -fuse-ld=gold works before enabling it.
  2. mysql abi check fails with the default Linux stdenv due to nostdinc/nolibc flags not respected in wrapper-cc #44530. It is not obvious that we should fix that and it is not fixed yet, so I have included a workaround for mysql.
  3. mysqlrouter fails to build on Darwin because it uses std::shared_timed_mutex that requires MACOSX_DEPLOYMENT_TARGET to be at least 10.12. I have set it, but disable mysqlrouter anyway because it is difficult to package it properly (it may require changing INSTALL_LAYOUT from STANDALONE to something else), and because it seems possible to package mysqlrouter separately from mysql.

@orivej-nixos orivej-nixos changed the title mysql80: init 8.0.17 mysql80: init at 8.0.17 Aug 11, 2019
@totten
Copy link
Contributor Author

totten commented Aug 12, 2019

Looks nicer!

I did a local sniff test with the latest revision - built on OSX, launched mysqld manually, connected with mysql, and created a dummy DB+table. That all went well. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants