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

set correct CWD for node processes #3746

Merged
merged 4 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions crates/next-core/js/src/entry/server-edge-api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ async function runEdgeFunction({
}): Promise<FetchEventResult | null> {
const edgeInfo = {
name: "edge",
paths: chunkGroup.map((chunk: string) => join(process.cwd(), chunk)),
paths: chunkGroup.map((chunk: string) =>
join(process.cwd(), ".next/server/pages", chunk)
),
wasm: [],
env: [],
assets: [],
Expand All @@ -77,7 +79,7 @@ async function runEdgeFunction({

const { run } = require("next/dist/server/web/sandbox");
const result = (await run({
distDir: process.cwd(),
distDir: join(process.cwd(), ".next/server/pages"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we pass this in as a param instead of duplicating the rust path?

Copy link
Contributor Author

@ForsakenHarmony ForsakenHarmony Feb 16, 2023

Choose a reason for hiding this comment

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

Maybe, but it would require some bigger code changes, and this is probably unlikely to change?

name: edgeInfo.name,
paths: edgeInfo.paths,
env: edgeInfo.env,
Expand Down
1 change: 1 addition & 0 deletions crates/next-core/src/app_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ async fn create_app_source_for_directory(
let params_matcher = NextParamsMatcherVc::new(pathname);

sources.push(create_node_rendered_source(
project_path,
specificity,
server_root,
params_matcher.into(),
Expand Down
26 changes: 15 additions & 11 deletions crates/next-core/src/page_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ pub async fn create_page_source(
/// Handles a single page file in the pages directory
#[turbo_tasks::function]
async fn create_page_source_for_file(
context_path: FileSystemPathVc,
project_path: FileSystemPathVc,
server_context: AssetContextVc,
server_data_context: AssetContextVc,
client_context: AssetContextVc,
Expand All @@ -320,7 +320,7 @@ async fn create_page_source_for_file(
);

let server_chunking_context = DevChunkingContextVc::builder(
context_path,
project_path,
intermediate_output_path,
intermediate_output_path.join("chunks"),
get_client_assets_path(
Expand All @@ -334,7 +334,7 @@ async fn create_page_source_for_file(
let data_intermediate_output_path = intermediate_output_path.join("data");

let server_data_chunking_context = DevChunkingContextVc::builder(
context_path,
project_path,
data_intermediate_output_path,
data_intermediate_output_path.join("chunks"),
get_client_assets_path(
Expand All @@ -346,7 +346,7 @@ async fn create_page_source_for_file(
.build();

let client_chunking_context = get_client_chunking_context(
context_path,
project_path,
server_root,
client_context.environment(),
Value::new(ClientContextType::Pages { pages_dir }),
Expand All @@ -364,6 +364,7 @@ async fn create_page_source_for_file(
SsrType::Api
};
create_node_api_source(
project_path,
specificity,
server_root,
pathname,
Expand Down Expand Up @@ -408,6 +409,7 @@ async fn create_page_source_for_file(

CombinedContentSourceVc::new(vec![
create_node_rendered_source(
project_path,
specificity,
server_root,
route_matcher.into(),
Expand All @@ -417,6 +419,7 @@ async fn create_page_source_for_file(
fallback_page,
),
create_node_rendered_source(
project_path,
specificity,
server_root,
data_route_matcher.into(),
Expand Down Expand Up @@ -454,7 +457,7 @@ async fn get_not_found_page(
/// Handles a single page file in the pages directory
#[turbo_tasks::function]
async fn create_not_found_page_source(
context_path: FileSystemPathVc,
project_path: FileSystemPathVc,
server_context: AssetContextVc,
client_context: AssetContextVc,
pages_dir: FileSystemPathVc,
Expand All @@ -467,7 +470,7 @@ async fn create_not_found_page_source(
route_matcher: RouteMatcherVc,
) -> Result<ContentSourceVc> {
let server_chunking_context = DevChunkingContextVc::builder(
context_path,
project_path,
intermediate_output_path,
intermediate_output_path.join("chunks"),
get_client_assets_path(
Expand All @@ -479,7 +482,7 @@ async fn create_not_found_page_source(
.build();

let client_chunking_context = get_client_chunking_context(
context_path,
project_path,
server_root,
client_context.environment(),
Value::new(ClientContextType::Pages { pages_dir }),
Expand All @@ -494,7 +497,7 @@ async fn create_not_found_page_source(
// The error page asset must be within the context path so it can depend on the
// Next.js module.
next_asset(
attached_next_js_package_path(context_path).join("entry/error.tsx"),
attached_next_js_package_path(project_path).join("entry/error.tsx"),
"entry/error.tsx",
),
// If no 404 page is defined, the pathname should be _error.
Expand Down Expand Up @@ -528,6 +531,7 @@ async fn create_not_found_page_source(

Ok(CombinedContentSourceVc::new(vec![
create_node_rendered_source(
project_path,
specificity,
server_root,
route_matcher,
Expand All @@ -546,7 +550,7 @@ async fn create_not_found_page_source(
/// [create_page_source_for_file] method for files.
#[turbo_tasks::function]
async fn create_page_source_for_directory(
context_path: FileSystemPathVc,
project_path: FileSystemPathVc,
server_context: AssetContextVc,
server_data_context: AssetContextVc,
client_context: AssetContextVc,
Expand Down Expand Up @@ -595,7 +599,7 @@ async fn create_page_source_for_directory(
sources.push((
name,
create_page_source_for_file(
context_path,
project_path,
server_context,
server_data_context,
client_context,
Expand All @@ -618,7 +622,7 @@ async fn create_page_source_for_directory(
sources.push((
name,
create_page_source_for_directory(
context_path,
project_path,
server_context,
server_data_context,
client_context,
Expand Down
19 changes: 14 additions & 5 deletions crates/turbo-tasks-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use self::{json::UnparseableJson, mutex_map::MutexMap};
#[cfg(target_family = "windows")]
use crate::util::is_windows_raw_path;
use crate::{
attach::AttachedFileSystemVc,
retry::{retry_blocking, retry_future},
rope::{Rope, RopeReadRef, RopeReader},
};
Expand Down Expand Up @@ -1666,12 +1667,20 @@ impl ValueToString for NullFileSystem {
}
}

pub async fn to_sys_path(path: FileSystemPathVc) -> Result<Option<PathBuf>> {
if let Some(fs) = DiskFileSystemVc::resolve_from(path.fs()).await? {
let sys_path = fs.await?.to_sys_path(path).await?;
return Ok(Some(sys_path));
pub async fn to_sys_path(mut path: FileSystemPathVc) -> Result<Option<PathBuf>> {
loop {
ForsakenHarmony marked this conversation as resolved.
Show resolved Hide resolved
if let Some(fs) = AttachedFileSystemVc::resolve_from(path.fs()).await? {
Copy link
Member

Choose a reason for hiding this comment

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

That's a bit nasty... We need to get rid of the attached file system...

path = fs.get_inner_fs_path(path);
continue;
}

if let Some(fs) = DiskFileSystemVc::resolve_from(path.fs()).await? {
let sys_path = fs.await?.to_sys_path(path).await?;
return Ok(Some(sys_path));
}

return Ok(None);
}
Ok(None)
}

pub fn register() {
Expand Down
21 changes: 11 additions & 10 deletions crates/turbopack-node/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ use std::{
path::PathBuf,
};

use anyhow::{anyhow, bail, Result};
use anyhow::{bail, Result};
use futures::{stream::FuturesUnordered, TryStreamExt};
use indexmap::IndexSet;
pub use node_entry::{
NodeEntry, NodeEntryVc, NodeRenderingEntriesVc, NodeRenderingEntry, NodeRenderingEntryVc,
};
use serde::{Deserialize, Serialize};
use serde_json::Value as JsonValue;
use turbo_tasks::{CompletionVc, CompletionsVc, TryJoinIterExt};
use turbo_tasks::{CompletionVc, CompletionsVc, TryJoinIterExt, ValueToString};
use turbo_tasks_fs::{to_sys_path, File, FileContent, FileSystemPathVc};
use turbopack_core::{
asset::{Asset, AssetVc, AssetsSetVc},
Expand Down Expand Up @@ -182,6 +182,7 @@ async fn separate_assets(
/// Creates a node.js renderer pool for an entrypoint.
#[turbo_tasks::function]
pub async fn get_renderer_pool(
cwd: FileSystemPathVc,
intermediate_asset: AssetVc,
intermediate_output_path: FileSystemPathVc,
output_root: FileSystemPathVc,
Expand All @@ -205,16 +206,16 @@ pub async fn get_renderer_pool(

emit(intermediate_asset, output_root).await?;

let cwd = output_root;
let entrypoint = intermediate_output_path.join("index.js");

if let (Some(cwd), Some(entrypoint)) = (to_sys_path(cwd).await?, to_sys_path(entrypoint).await?)
{
let pool = NodeJsPool::new(cwd, entrypoint, HashMap::new(), 4, debug);
Ok(pool.cell())
} else {
Err(anyhow!("can only render from a disk filesystem"))
}
let Some(cwd) = to_sys_path(cwd).await? else {
bail!("can only render from a disk filesystem, but `cwd = {}`", cwd.fs().to_string().await?);
};
let Some(entrypoint) = to_sys_path(entrypoint).await? else {
bail!("can only render from a disk filesystem, but `entrypoint = {}`", entrypoint.fs().to_string().await?);
};

Ok(NodeJsPool::new(cwd, entrypoint, HashMap::new(), 4, debug).cell())
}

/// Converts a module graph into node.js executable assets
Expand Down
14 changes: 9 additions & 5 deletions crates/turbopack-node/src/render/node_api_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use crate::{
/// Creates a [NodeApiContentSource].
#[turbo_tasks::function]
pub fn create_node_api_source(
cwd: FileSystemPathVc,
specificity: SpecificityVc,
server_root: FileSystemPathVc,
pathname: StringVc,
Expand All @@ -31,6 +32,7 @@ pub fn create_node_api_source(
runtime_entries: EcmascriptChunkPlaceablesVc,
) -> ContentSourceVc {
NodeApiContentSource {
cwd,
specificity,
server_root,
pathname,
Expand All @@ -50,6 +52,7 @@ pub fn create_node_api_source(
/// to this directory.
#[turbo_tasks::value]
pub struct NodeApiContentSource {
cwd: FileSystemPathVc,
specificity: SpecificityVc,
server_root: FileSystemPathVc,
pathname: StringVc,
Expand Down Expand Up @@ -114,8 +117,8 @@ impl GetContentSourceContent for NodeApiGetContentResult {
}
#[turbo_tasks::function]
async fn get(&self, data: Value<ContentSourceData>) -> Result<ContentSourceContentVc> {
let this = self.source.await?;
let Some(params) = &*this.route_match.params(&self.path).await? else {
let source = self.source.await?;
let Some(params) = &*source.route_match.params(&self.path).await? else {
return Err(anyhow!("Non matching path provided"));
};
let ContentSourceData {
Expand All @@ -128,11 +131,12 @@ impl GetContentSourceContent for NodeApiGetContentResult {
} = &*data else {
return Err(anyhow!("Missing request data"));
};
let entry = this.entry.entry(data.clone()).await?;
let entry = source.entry.entry(data.clone()).await?;
Ok(ContentSourceContent::HttpProxy(render_proxy(
this.server_root.join(&self.path),
source.cwd,
source.server_root.join(&self.path),
entry.module,
this.runtime_entries,
source.runtime_entries,
entry.chunking_context,
entry.intermediate_output_path,
entry.output_root,
Expand Down
10 changes: 7 additions & 3 deletions crates/turbopack-node/src/render/render_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::{get_intermediate_asset, get_renderer_pool, pool::NodeJsOperation, tr
/// Renders a module as static HTML in a node.js process.
#[turbo_tasks::function]
pub async fn render_proxy(
cwd: FileSystemPathVc,
path: FileSystemPathVc,
module: EcmascriptModuleAssetVc,
runtime_entries: EcmascriptChunkPlaceablesVc,
Expand All @@ -27,13 +28,16 @@ pub async fn render_proxy(
module.as_evaluated_chunk(chunking_context, Some(runtime_entries)),
intermediate_output_path,
);
let renderer_pool = get_renderer_pool(

let pool = get_renderer_pool(
cwd,
intermediate_asset,
intermediate_output_path,
output_root,
/* debug */ false,
);
let pool = renderer_pool.await?;
)
.await?;

let mut operation = match pool.operation().await {
Ok(operation) => operation,
Err(err) => {
Expand Down
2 changes: 2 additions & 0 deletions crates/turbopack-node/src/render/render_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ impl StaticResultVc {
/// Renders a module as static HTML in a node.js process.
#[turbo_tasks::function]
pub async fn render_static(
cwd: FileSystemPathVc,
path: FileSystemPathVc,
module: EcmascriptModuleAssetVc,
runtime_entries: EcmascriptChunkPlaceablesVc,
Expand All @@ -61,6 +62,7 @@ pub async fn render_static(
intermediate_output_path,
);
let renderer_pool = get_renderer_pool(
cwd,
intermediate_asset,
intermediate_output_path,
output_root,
Expand Down
Loading