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

shell: make sure proper .env is loaded in script runner #9448

Closed
wants to merge 4 commits into from

Conversation

nektro
Copy link
Contributor

@nektro nektro commented Mar 15, 2024

Closes #9445

@nektro nektro requested a review from zackradisic March 15, 2024 23:05
Copy link

github-actions bot commented Mar 15, 2024

❌🪟 @Jarred-Sumner, there are 18 test regressions on Windows x86_64

  • test\cli\install\bun-run-bunfig.test.ts
  • test\cli\install\migration\migrate.test.ts
  • test\cli\run\run-cjs.test.ts
  • test\cli\run\run-process-env.test.ts
  • test\cli\run\run-eval.test.ts
  • test\cli\run\env.test.ts
  • test\cli\run\require-cache.test.ts
  • test\cli\watch\watch.test.ts
  • test\cli\run\transpiler-cache.test.ts
  • test\js\bun\http\bun-server.test.ts
  • test\js\bun\http\fetch-file-upload.test.ts
  • test\js\bun\spawn\spawn_waiter_thread.test.ts
  • test\js\node\dns\node-dns.test.js
  • test\js\third_party\es-module-lexer\es-module-lexer.test.ts
  • test\js\node\watch\fs.watchFile.test.ts
  • test\js\web\websocket\websocket.test.js
  • test\js\web\fetch\body-stream.test.ts
  • test\regression\issue\09340.test.ts

Full Test Output

const { stdout, stderr, exited } = Bun.spawn({
cmd: [bunExe(), "run", "prod"],
cwd: package_dir,
stdio: ["pipe", "pipe", "pipe"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
stdio: ["pipe", "pipe", "pipe"],
stdio: ["ignore", "pipe", "pipe"],

const { stdout, stderr, exited } = Bun.spawn({
cmd: [bunExe(), "prod"],
cwd: package_dir,
stdio: ["pipe", "pipe", "pipe"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
stdio: ["pipe", "pipe", "pipe"],
stdio: ["ignore", "pipe", "pipe"],

await writeFile(join(package_dir, ".env.development"), `AWESOME=development`);

const { stdout, stderr, exited } = Bun.spawn({
cmd: [bunExe(), "run", "prod"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
cmd: [bunExe(), "run", "prod"],
cmd: [bunExe(), "--shell=bun", "run", "prod"],

@@ -831,3 +832,61 @@ function sentinelByte(buf: Uint8Array): number {
}
throw new Error("No sentinel byte");
}

test("shell as script runner propagates env vars: bun run <foo>", async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be condensed into a test.each for the bun run permutation and the non-bun run permutation

@Jarred-Sumner
Copy link
Collaborator

I think this bug is actually the shell not loading environment variables, and this fix causes a different bug where it no longer loads .env files in bun run at the right time

@paperdave
Copy link
Collaborator

superceded by #9642

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.

environment variable loading is broken with 1.0.31
4 participants