-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix: re-install lmdb when detecting the absence of @LMDB prebuilt packages #38691
Changes from 4 commits
67c71f3
9e50274
619abf1
c3cd62b
22be691
2555ba9
ae45348
965db45
7c0e053
01bf938
915dbe0
f2deaac
9c23192
8d793e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,8 +2,10 @@ | |||||||||||
|
||||||||||||
import * as path from "path" | ||||||||||||
import * as fs from "fs-extra" | ||||||||||||
import execa, { Options as ExecaOptions } from "execa" | ||||||||||||
import webpack, { Module, NormalModule, Compilation } from "webpack" | ||||||||||||
import ConcatenatedModule from "webpack/lib/optimize/ConcatenatedModule" | ||||||||||||
import { dependencies } from "gatsby/package.json" | ||||||||||||
import { printQueryEnginePlugins } from "./print-plugins" | ||||||||||||
import mod from "module" | ||||||||||||
import { WebpackLoggingPlugin } from "../../utils/webpack/plugins/webpack-logging" | ||||||||||||
|
@@ -35,6 +37,71 @@ function getApisToRemoveForQueryEngine(): Array<GatsbyNodeAPI> { | |||||||||||
return apisToRemove | ||||||||||||
} | ||||||||||||
|
||||||||||||
const getInternalPackagesCacheDir = (): string => | ||||||||||||
path.join(process.cwd(), `.cache/internal-packages`) | ||||||||||||
|
||||||||||||
// Create a directory and JS module where we install internally used packages | ||||||||||||
const createInternalPackagesCacheDir = async (): Promise<void> => { | ||||||||||||
const cacheDir = getInternalPackagesCacheDir() | ||||||||||||
await fs.ensureDir(cacheDir) | ||||||||||||
await fs.emptyDir(cacheDir) | ||||||||||||
|
||||||||||||
const packageJsonPath = path.join(cacheDir, `package.json`) | ||||||||||||
|
||||||||||||
await fs.outputJson(packageJsonPath, { | ||||||||||||
name: `gatsby-internal-packages`, | ||||||||||||
description: `This directory contains internal packages installed by Gatsby used to comply with the current platform requirements`, | ||||||||||||
version: `1.0.0`, | ||||||||||||
private: true, | ||||||||||||
author: `Gatsby`, | ||||||||||||
license: `MIT`, | ||||||||||||
}) | ||||||||||||
} | ||||||||||||
|
||||||||||||
// Detect if the prebuilt binaries for lmdb have been installed. These are installed under @lmdb and are tied to each platform/arch. We've seen instances where regular installations lack these modules because of a broken lockfile or skipping optional dependencies installs | ||||||||||||
function lmdbPrebuiltPackagePresent(): boolean { | ||||||||||||
try { | ||||||||||||
require.resolve(`@lmdb/lmdb-${process.platform}-${process.arch}`) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In some cases this would not resolve even if it's actually installed when we try to Yarn PnP wouldn't allow that because those prebuilt packages are not dependencies of gatsby and are transitive dependency and you can't require/import transitive deps that you package doesn't declare as deps. In
this would also mean it doesn't resolve. For this check to work without relying on
Suggested change
The other part is that I'm not sure about is if there are prebuilts for all the platform+arch combinations that exist. If prebuilt for the combo doesn't exist we will always try to install
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Greate suggestions, I'll add those in 👍 |
||||||||||||
return true | ||||||||||||
} catch (e) { | ||||||||||||
return false | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
// Install lmdb under our internal cache if we detect the current installation | ||||||||||||
// isn't using the pre-build binaries | ||||||||||||
async function installIfMissingLmdb(): Promise<string | undefined> { | ||||||||||||
if (lmdbPrebuiltPackagePresent()) return undefined | ||||||||||||
|
||||||||||||
await createInternalPackagesCacheDir() | ||||||||||||
|
||||||||||||
const cacheDir = getInternalPackagesCacheDir() | ||||||||||||
const options: ExecaOptions = { | ||||||||||||
stderr: `inherit`, | ||||||||||||
cwd: cacheDir, | ||||||||||||
} | ||||||||||||
|
||||||||||||
const npmAdditionalCliArgs = [ | ||||||||||||
`--no-progress`, | ||||||||||||
`--no-audit`, | ||||||||||||
`--no-fund`, | ||||||||||||
`--loglevel`, | ||||||||||||
`error`, | ||||||||||||
`--color`, | ||||||||||||
`always`, | ||||||||||||
`--legacy-peer-deps`, | ||||||||||||
`--save-exact`, | ||||||||||||
] | ||||||||||||
|
||||||||||||
await execa( | ||||||||||||
`npm`, | ||||||||||||
[`install`, ...npmAdditionalCliArgs, `lmdb@${dependencies.lmdb}`], | ||||||||||||
options | ||||||||||||
) | ||||||||||||
|
||||||||||||
return path.join(cacheDir, `node_modules`, `lmdb`) | ||||||||||||
} | ||||||||||||
|
||||||||||||
export async function createGraphqlEngineBundle( | ||||||||||||
rootDir: string, | ||||||||||||
reporter: Reporter, | ||||||||||||
|
@@ -57,6 +124,9 @@ export async function createGraphqlEngineBundle( | |||||||||||
require.resolve(`gatsby-plugin-typescript`) | ||||||||||||
) | ||||||||||||
|
||||||||||||
// Alternative lmdb path we've created to self heal from a "broken" lmdb installation | ||||||||||||
const alternativeLmdbPath = await installIfMissingLmdb() | ||||||||||||
|
||||||||||||
const compiler = webpack({ | ||||||||||||
name: `Query Engine`, | ||||||||||||
// mode: `production`, | ||||||||||||
|
@@ -121,6 +191,7 @@ export async function createGraphqlEngineBundle( | |||||||||||
{ | ||||||||||||
loader: require.resolve(`./lmdb-bundling-patch`), | ||||||||||||
options: { | ||||||||||||
alternativeLmdbPath, | ||||||||||||
forcedBinaryModule: store.getState().adapter.instance | ||||||||||||
? `@lmdb/lmdb-${process.platform}-${process.arch}/node.abi83.glibc.node` | ||||||||||||
: undefined, | ||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do wonder if there is something we could do to "sandbox" this to not have potential repo/site level
.npmrc
(or even user level.npmrc
locally) potentially cause problems with installing optional packages. If there is.npmrc
withomit=optional
(or however the syntax it is) - that would use in this directory as well?Alternatively, maybe instead of installing whole
lmdb
(and let that install optional packages with prebuilds) - maybe we try to install those prebuild package directly? Maybe then it would be less prone to inherit same problem that cause those not to be installed normally in the first place?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess what I wonder is, in the presence of
@lmdb/x
package will that always be favoured even if there's abuild
from source in thelmdb
directory? (I can dig through the code to try to understand that but tbh I worry that we might miss something by doing so)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like general order of loading is to first try to load from
build/Release
first before trying optional packages. However this relies on fact that there isinstall
script that before trying to compile from source check if there is binary that can be loaded - and when prebuilts are installed - it finds those and that check prevent from even compiling from source ( https://unpkg.com/browse/node-gyp-build-optional-packages/ is doing all those things ).However - our
lmdb-bundling-patch
replace runtime usage ofnode-gyp-build(-optional-packages)
fromlmdb
and replace that with direct path to.node
file, so we are not subject to regular behavior when bundling for lambdas as we get rid of it here:gatsby/packages/gatsby/src/schema/graphql-engine/lmdb-bundling-patch.ts
Lines 74 to 85 in 22c2412
So I think if we can install just one of prebuild packages like
@lmdb/lmdb-linux-x64
and get path to.node
file and make the lmdb bundling patch use that - I think this should be fine (?)Also when we install whole
lmdb
in.cache/internal-packages
, we don't actually uselmdb
instance from there currently - we could add webpack alias to use that instance oflmdb
package for lambda, but my biggest hang up with it is that installing mainlmdb
package it in nested directory could inherit any configuration that caused those prebuilt packages to not be installed in first place.PS. after going through
node-gyp-build-optional-packages
I also found that:in
.npmrc
would also likely cause the issue (similarly toomit = optional
)