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

chore(MONGOSH-1808): migrate mongosh to new mongodb-client-encryption repository #2023

Closed
wants to merge 17 commits into from

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Jun 11, 2024

Changes prep-fle-addon script to use new repo and depend on new repo's script for downloading/building bindings. The version of libmongocrypt is now determined by the binding's package.json file.

@nbbeeken nbbeeken added the wip Work in Progress label Jun 11, 2024
@nbbeeken
Copy link
Contributor Author

v6.0.0 and v6.0.1 tags in the other repo do not have a working version of the download/build script. This PR should be picked up once we have tagged and released from the new repo.

@nbbeeken nbbeeken changed the title chore: migrate mongosh to new mongodb-client-encryption repository chore(MONGOSH-1808): migrate mongosh to new mongodb-client-encryption repository Jun 18, 2024
@nbbeeken
Copy link
Contributor Author

nbbeeken commented Jun 18, 2024

Currently blocked on (evg patch):

@addaleax
Copy link
Contributor

  • Not sure what's going on there, it is during the boxednode build. FLE completes its build here Where would the arch flag be modified after that?

I think it's not complaining about the value of -arch, it doesn't seem to understand the -arch flag as all and rather interpret it as -a -r -c -h (and doesn't know -r specifically).

These flags are coming from here: https://github.com/mongodb-js/mongodb-client-encryption/blob/95849ddca9a452b1f697189a29a483d354a394bd/binding.gyp#L27-L33 – they should probably be in the build_type=="dynamic" sub-section?

CMake 3.12 or higher is required. You are running version 3.11.0

Fwiw, we happily adjust our build PATH as needed in .evergreen/setup-env.sh 🙂

@nbbeeken
Copy link
Contributor Author

Good call on the -arch flag being split up. I'll look into that, but I don't think that it should be in the "dynamic" section, IIUC that is unused, we build static prebuilds and we're trying to build statically for mongosh here.

It would be nice if I could somehow feature detect, but maybe I can add a new variable we pipe in for our prebuilds to set -arch and otherwise it is omitted. I will investigate further.

@nbbeeken
Copy link
Contributor Author

An update, so I've enabled the ability to only build for the specific macos platform, however, even when only building for the local platform node-gyp is still putting -arch into the build flags which is making libtool unhappy on BigSur, maybe we've been using a newer version of node-gyp or some other tool that makes an assumption about support for that flag.

Window, still need to find a newer cmake version, haven't started on that. I don't want to have to download it but may have to.

@nbbeeken nbbeeken marked this pull request as ready for review June 28, 2024 18:10
@nbbeeken nbbeeken removed the wip Work in Progress label Jun 28, 2024
@nbbeeken nbbeeken requested a review from addaleax June 28, 2024 18:11
@nbbeeken
Copy link
Contributor Author

A lot of red but none of them are compile_artifact 🎉

@mabaasit
Copy link
Contributor

Closing in favour of #2076

@mabaasit mabaasit closed this Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants