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

mongodb: fixes build and sanitize package #172009

Closed
wants to merge 7 commits into from

Conversation

bryanasdev000
Copy link
Member

@bryanasdev000 bryanasdev000 commented May 8, 2022

Description of changes

This should close #171928 and #155121.

Basically a umbrella PR to fix MongoDB issues on Nixpkgs.

With help from @andersk @bachp @kfiz @superherointj @Artturin, thanks a lot!

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@bryanasdev000 bryanasdev000 changed the title mongodb: fixes build mongodb: fixes build and sanitize packages May 8, 2022
@bryanasdev000 bryanasdev000 changed the title mongodb: fixes build and sanitize packages mongodb: fixes build and sanitize package May 8, 2022
@kfiz
Copy link

kfiz commented May 9, 2022

Please change this
stdenv = gcc10Stdenv;
in all-packages.nix
to
stdenv = if stdenv.cc.isGNU then gcc10Stdenv else stdenv;
or otherwise we run into problems on non-linux

@bryanasdev000
Copy link
Member Author

stdenv = if stdenv.cc.isGNU then gcc10Stdenv else stdenv;

Done!

@bryanasdev000
Copy link
Member Author

Folks, I think it's ready to ship, a squash is necessary?

I am just waiting for ofborg output before removing the draft.

CC: @andersk @bachp @kfiz @superherointj

@kfiz
Copy link

kfiz commented May 9, 2022

Folks, I think it's ready to ship, a squash is necessary?

I am just waiting for ofborg output before removing the draft.

CC: @andersk @bachp @kfiz @superherointj

LGTM. To nitpick, there is a minor typo in the doc sections: "provider" -> "provide".

@bryanasdev000
Copy link
Member Author

provider

Fixed, will probably need to squash :P

@bryanasdev000 bryanasdev000 marked this pull request as ready for review May 9, 2022 15:51
@thiagokokada
Copy link
Contributor

@ofborg eval

@bryanasdev000
Copy link
Member Author

Need to fix top level entry...

@bryanasdev000
Copy link
Member Author

So, I pushed a mess of tests, so far, MongoDB 4.4 and 5.0.9 still fails to build.

Will try it again when less sleepy, gist for logs https://gist.github.com/bryanasdev000/b086bbf9c9a0f278b9f6eb4b1670549e

@bryanasdev000
Copy link
Member Author

bryanasdev000 commented Jul 4, 2022

Will try to build PR #178820 over master to check if it builds and what's the difference that's causing it to fail.

EDIT: Fails too https://gist.github.com/bryanasdev000/b086bbf9c9a0f278b9f6eb4b1670549e#file-5-0-7-txt

@Et7f3
Copy link
Contributor

Et7f3 commented Jul 7, 2022

I tested on a Mac builder (with better CPU and more disk). It was also broken. It seem the issue was boost 1.79 you might be interested in cherry-pick #180620

@bryanasdev000
Copy link
Member Author

I tested on a Mac builder (with better CPU and more disk). It was also broken. It seem the issue was boost 1.79 you might be interested in cherry-pick #180620

Sorry for the delay @Et7f3, been busy with IRL.

I've tried to build 4.4, and I am leaving 5.0 in the queue to build on my laptop, but so far 4.4 fails with iostream errors on your branch and another full set of errors with cherry-picking (sadly did not catch the full log).

If mongo 4.4 and/or 5.0 is working for you on Darwin, I suggest that you unify your PRs (#178820, #178820 and any other necessary piece to make it fully work) and push it to review, so this PR will not be a blocker. Also, I can give some help in code review at least.

Note that I only have access to x86-64 architectures, so we may be chasing different errors :P

@Et7f3
Copy link
Contributor

Et7f3 commented Jul 13, 2022

I don't think I will merge my two PR since it is targeted for 2 users classes. (Maybe their is overlap). From your logs it seem mongodb include special file only for linux hence it didn't caused issue on the darwin build.

@Et7f3
Copy link
Contributor

Et7f3 commented Jul 20, 2022

Ok I did a round on linux and finally merged the 2 PR (since their is a overlap). See my PR for darwin and it should work for darwin and linux. (I am currently rebuilding after a rebase to be sure nothing got broken in the 2k commits)

sasl = cyrus_sasl;
boost = boost17x.override { enableShared = false; };
inherit (darwin) cctools;
inherit (darwin.apple_sdk.frameworks) CoreFoundation Security;
};

mongodb-5_0 = callPackage ../servers/nosql/mongodb/5.0.nix {
mongodb-5_0 = callPackage ../servers/nosql/mongodb/v5_0.nix {
sasl = cyrus_sasl;
Copy link
Member

Choose a reason for hiding this comment

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

Do we even need this? It seems always to be cyrus_sasl.

Copy link

Choose a reason for hiding this comment

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

Would it be clear which sasl library should be used if we removed it? There seem to be at least 2 options, namely cyrus_sasl and gsasl.

Copy link
Member

Choose a reason for hiding this comment

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

Does mongodb even work with gsasl? If so this should rather be a boolean input to enable one or the other.

Copy link

Choose a reason for hiding this comment

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

@kfiz
Copy link

kfiz commented Sep 11, 2022

Ok I did a round on linux and finally merged the 2 PR (since their is a overlap). See my PR for darwin and it should work for darwin and linux. (I am currently rebuilding after a rebase to be sure nothing got broken in the 2k commits)

@Et7f3 @bryanasdev000 Should we try to wrap this up?

@Et7f3
Copy link
Contributor

Et7f3 commented Sep 11, 2022

I tested my PR #180620 on both linux and mac 2 months ago and all builded. I didn't retested recently but it should still be in good shape. If you @kfiz mind reviewing. It is a full PR done.

@bryanasdev000
Copy link
Member Author

I am away from my computer now, but I can try to dust it off and try again once I get back home.

We have 3 PRs now trying to fix Mongo at nixpkgs, this, @Et7f3 one and #190548 maybe now we can fix it for real.

@bryanasdev000
Copy link
Member Author

I tested my PR #180620 on both linux and mac 2 months ago and all builded. I didn't retested recently but it should still be in good shape. If you @kfiz mind reviewing. It is a full PR done.

@Et7f3 since you have hardware for it, could you please retest atop of master? Just in case, to test Darwin specifics.

@kfiz
Copy link

kfiz commented Sep 11, 2022

I tested my PR #180620 on both linux and mac 2 months ago and all builded. I didn't retested recently but it should still be in good shape. If you @kfiz mind reviewing. It is a full PR done.

I tested the v5 patch on darwin and that worked. I'll have a look at the other adjustments later, but would give @bryanasdev000 the lead here as he has the best overview.

If we think we are good to go, we can consider updating the v5 build to 5.0.12 which is the latest patch release. I have a version of that here kfiz@b48aa95

Comment on lines +128 to +130
mkDefault (if versionAtLeast config.system.stateVersion "22.11" then pkgs.mongodb-5_0
else if versionAtLeast config.system.stateVersion "22.05" then pkgs.mongodb-3_4
else throw "please set your desired package for state version ${config.system.stateVersion}");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mkDefault (if versionAtLeast config.system.stateVersion "22.11" then pkgs.mongodb-5_0
else if versionAtLeast config.system.stateVersion "22.05" then pkgs.mongodb-3_4
else throw "please set your desired package for state version ${config.system.stateVersion}");
mkDefault (if versionAtLeast config.system.stateVersion "22.11" then pkgs.mongodb-5_0
else pkgs.mongodb-3_4;

@@ -32,8 +32,7 @@ in
enable = mkEnableOption "the MongoDB server";

package = mkOption {
default = pkgs.mongodb;
Copy link
Member

Choose a reason for hiding this comment

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

We should keep a default package

Copy link
Member Author

Choose a reason for hiding this comment

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

But this option will be overridden with the condition of stateVersion (a little bit below this snippet), so why define a version here? Also, if defined, won't conflict with this same condition?

@@ -92,6 +92,8 @@ In addition to numerous new and upgraded packages, this release has the followin
as it requires `qt4`, which reached its end-of-life 2015 and will no longer be supported by nixpkgs.
[According to Barco](https://www.barco.com/de/support/knowledge-base/4380-can-i-use-linux-os-with-clickshare-base-units) many of their base unit models can be used with Google Chrome and the Google Cast extension.

- MongoDB's packages were updated to each latest major version and versions older than 4.0 marked with `meta.known Vulnerabilities` to flag as EOLed by upstream yet still in tree to provide a clean upgrade path, also MongoDB 5.x is the default version from now on.
Copy link
Member

@SuperSandro2000 SuperSandro2000 Sep 11, 2022

Choose a reason for hiding this comment

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

Suggested change
- MongoDB's packages were updated to each latest major version and versions older than 4.0 marked with `meta.known Vulnerabilities` to flag as EOLed by upstream yet still in tree to provide a clean upgrade path, also MongoDB 5.x is the default version from now on.
- MongoDB's packages were updated to each latest major version and versions older than 4.0 marked with `meta.knownVulnerabilities` to flag as EOLed by upstream yet still in tree to provide a clean upgrade path, also MongoDB 5.x is the default version from now on.

that is not quite correct. meta.knownVulnerabilities has nothing to do with the maintenance status of upstream bit that the package contains long standing not fixed vulnerabilities which is the case because those packages are EOL.

Also what is the upgrade path for mongodb? Do we really need to keep 4.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what we found to mark EOL and give space to users to do a clean upgrade from 3.4 to the latest (be it 5.X or 6.X if #190548 gets merged first).

There is another Nix mechanism to do that?

And yes we need to keep all versions, as one should upgrade version by version until it reaches the desired one, see https://www.mongodb.com/docs/manual/release-notes/5.0-upgrade-standalone/ and #155121 (comment) for reference.

Besides that, I think another way is doing the migration with a container then once the data is OK, start the service in NixOS with the desired version.

@@ -133,7 +133,7 @@ in stdenv.mkDerivation rec {
description = "A scalable, high-performance, open source NoSQL database";
homepage = "http://www.mongodb.org";
inherit license;

knownVulnerabilities = if (versionAtLeast version "4.0") then [] else [ "EOLed version, please check https://www.mongodb.com/support-policy/lifecycles and official docs on how to upgrade" ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
knownVulnerabilities = if (versionAtLeast version "4.0") then [] else [ "EOLed version, please check https://www.mongodb.com/support-policy/lifecycles and official docs on how to upgrade" ];
knownVulnerabilities = lib.optionals (versionAtLeast version "4.0") [ "EOLed version, please check https://www.mongodb.com/support-policy/lifecycles and official docs on how to upgrade" ];

Please add concrete CVEs from https://www.mongodb.com/alerts#security into here. This field is not for marking EOL software without vulnerabilities.

Copy link
Member Author

Choose a reason for hiding this comment

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

Answered above.

@bryanasdev000
Copy link
Member Author

bryanasdev000 commented Sep 12, 2022

Ok, thanks to @Et7f3 and @kfiz we now should have all mongodb versions in tree building (with exception being the v6.x per #190548 (comment)) in Darwin, thanks a lot!

Now, I think we should merge PR #190548 first and then focus on this one with all needed decisions, that include:

  • Fix all builds on Linux
  • Chose what, when and how to deprecate EOL Mongo versions
  • Changelog
  • Fix conflicts
  • Release update (if any)
  • Refactors (if any)

This PR is also a little bit old and big, if needed we can split it in multiples, each one addressing some of the above points.

@kfiz
Copy link

kfiz commented Sep 12, 2022

This PR is also a little bit old and big, if needed we can split it in multiples, each one addressing some of the above points.
@SuperSandro2000 What would you suggest?

@Et7f3
Copy link
Contributor

Et7f3 commented Oct 12, 2022

mongodb v6.0 landed nixpkgs so we can revive this PR

@bryanasdev000
Copy link
Member Author

bryanasdev000 commented Oct 25, 2022

mongodb v6.0 landed nixpkgs so we can revive this PR

And thanks to @kfiz we should have MongoDB 6.x running on Darwin land soon.

I will try to check what is missing for close this PR in the weekend and maybe redo this PR or split it in multiple.

@dotlambda
Copy link
Member

Can this be closed?

@bryanasdev000
Copy link
Member Author

Can this be closed?

I think we can, AFAIK we have both old and new versions working and mongodb 6 as default pkg.

What will be missing is to deprecate old versions (#195448) but honestly I am without energy to pursue this.

Thanks for the help everyone, if anyone need a bit of this PR, feel free to grab.

This pull request was closed.
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.

mongodb: 4.0, 4.2, 4.4 and 5.0 broken at master