-
-
Notifications
You must be signed in to change notification settings - Fork 106
[codex] Fix ESM-safe config helper binstubs #1104
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
Changes from all commits
e8b625a
42ac154
aa58d0d
76d51d2
ff95c11
b66f6f6
21179d8
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 | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,64 +1,45 @@ | ||||||||||||||||||||
| #!/usr/bin/env node | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const { createRequire } = require("module") | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const PACK_CONFIG_DIFF_PACKAGE = "pack-config-diff" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function formatError(error) { | ||||||||||||||||||||
| return error instanceof Error ? error.message : String(error) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function exitWithCode(exitCode) { | ||||||||||||||||||||
| if (!Number.isInteger(exitCode)) { | ||||||||||||||||||||
| console.error( | ||||||||||||||||||||
| `[Shakapacker] ${PACK_CONFIG_DIFF_PACKAGE} returned a non-integer exit code: ${String(exitCode)}` | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| process.exit(2) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Forward only pack-config-diff's documented exit codes (0 = no diffs, 1 = diffs found). | ||||||||||||||||||||
| // Any other code is a tool failure — normalize to 2. | ||||||||||||||||||||
| process.exit(exitCode <= 1 ? exitCode : 2) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let run | ||||||||||||||||||||
|
|
||||||||||||||||||||
| try { | ||||||||||||||||||||
| // Resolve pack-config-diff via shakapacker's dependency tree so strict package | ||||||||||||||||||||
| // managers (pnpm, Yarn PnP) can find the transitive dependency. | ||||||||||||||||||||
| const shakapackerRequire = createRequire( | ||||||||||||||||||||
| require.resolve("shakapacker/package.json") | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| const loadedModule = shakapackerRequire("pack-config-diff") | ||||||||||||||||||||
| run = loadedModule.run | ||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||
| console.error( | ||||||||||||||||||||
| `[Shakapacker] Failed to load ${PACK_CONFIG_DIFF_PACKAGE}: ${formatError(error)}` | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| process.exit(2) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (typeof run !== "function") { | ||||||||||||||||||||
| console.error( | ||||||||||||||||||||
| `[Shakapacker] ${PACK_CONFIG_DIFF_PACKAGE} did not export a run() function` | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| process.exit(2) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| try { | ||||||||||||||||||||
| const runResult = run(process.argv.slice(2)) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (runResult && typeof runResult.then === "function") { | ||||||||||||||||||||
| runResult | ||||||||||||||||||||
| .then((exitCode) => exitWithCode(exitCode)) | ||||||||||||||||||||
| .catch((error) => { | ||||||||||||||||||||
| console.error(formatError(error)) | ||||||||||||||||||||
| process.exit(2) | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| exitWithCode(runResult) | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||
| console.error(formatError(error)) | ||||||||||||||||||||
| process.exit(2) | ||||||||||||||||||||
| } | ||||||||||||||||||||
| #!/usr/bin/env ruby | ||||||||||||||||||||
| # frozen_string_literal: true | ||||||||||||||||||||
|
|
||||||||||||||||||||
| # Keep in sync with lib/install/bin/shakapacker-config and the template in | ||||||||||||||||||||
| # package/configExporter/cli.ts (createBinStub). | ||||||||||||||||||||
| def shakapacker_app_root | ||||||||||||||||||||
| candidate = File.expand_path("..", __dir__) | ||||||||||||||||||||
| return candidate if File.exist?(File.join(candidate, "Gemfile")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| warn "[Shakapacker] No Gemfile found at #{candidate.inspect}; " \ | ||||||||||||||||||||
| "falling back to the current directory (#{Dir.pwd.inspect})." | ||||||||||||||||||||
| Dir.pwd | ||||||||||||||||||||
| end | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def shakapacker_node_binary | ||||||||||||||||||||
| node_bin = "node" | ||||||||||||||||||||
| return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \ | ||||||||||||||||||||
| "Install Node.js and try again." | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| end | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development" | ||||||||||||||||||||
| ENV["NODE_ENV"] ||= "development" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| app_root = shakapacker_app_root | ||||||||||||||||||||
| node_bin = shakapacker_node_binary | ||||||||||||||||||||
| script_path = File.join( | ||||||||||||||||||||
| app_root, | ||||||||||||||||||||
| "node_modules", | ||||||||||||||||||||
| "shakapacker", | ||||||||||||||||||||
| "package", | ||||||||||||||||||||
| "bin", | ||||||||||||||||||||
| "diff-bundler-config.cjs" | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| unless File.file?(script_path) | ||||||||||||||||||||
| warn "[Shakapacker] Could not find #{script_path}. Run your package manager install command and try again." | ||||||||||||||||||||
| exit 1 | ||||||||||||||||||||
| end | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Dir.chdir(app_root) do | ||||||||||||||||||||
| exec node_bin, script_path, *ARGV | ||||||||||||||||||||
| end | ||||||||||||||||||||
|
Comment on lines
+43
to
+45
Contributor
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.
Suggested change
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,11 +1,45 @@ | ||||||||||||||||||||||||||||||||||
| #!/usr/bin/env node | ||||||||||||||||||||||||||||||||||
| #!/usr/bin/env ruby | ||||||||||||||||||||||||||||||||||
| # frozen_string_literal: true | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Minimal shim - all logic is in the TypeScript module | ||||||||||||||||||||||||||||||||||
| const { run } = require("shakapacker/configExporter") | ||||||||||||||||||||||||||||||||||
| # Keep in sync with lib/install/bin/diff-bundler-config and the template in | ||||||||||||||||||||||||||||||||||
|
justin808 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
| # package/configExporter/cli.ts (createBinStub). | ||||||||||||||||||||||||||||||||||
| def shakapacker_app_root | ||||||||||||||||||||||||||||||||||
| candidate = File.expand_path("..", __dir__) | ||||||||||||||||||||||||||||||||||
| return candidate if File.exist?(File.join(candidate, "Gemfile")) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| run(process.argv.slice(2)) | ||||||||||||||||||||||||||||||||||
| .then((exitCode) => process.exit(exitCode)) | ||||||||||||||||||||||||||||||||||
| .catch((error) => { | ||||||||||||||||||||||||||||||||||
| console.error(error.message) | ||||||||||||||||||||||||||||||||||
| process.exit(1) | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| warn "[Shakapacker] No Gemfile found at #{candidate.inspect}; " \ | ||||||||||||||||||||||||||||||||||
| "falling back to the current directory (#{Dir.pwd.inspect})." | ||||||||||||||||||||||||||||||||||
| Dir.pwd | ||||||||||||||||||||||||||||||||||
|
justin808 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| def shakapacker_node_binary | ||||||||||||||||||||||||||||||||||
|
justin808 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
| node_bin = "node" | ||||||||||||||||||||||||||||||||||
| return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \ | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+19
Contributor
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. The
Suggested change
Not a blocker — the |
||||||||||||||||||||||||||||||||||
| "Install Node.js and try again." | ||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development" | ||||||||||||||||||||||||||||||||||
| ENV["NODE_ENV"] ||= "development" | ||||||||||||||||||||||||||||||||||
|
justin808 marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| app_root = shakapacker_app_root | ||||||||||||||||||||||||||||||||||
| node_bin = shakapacker_node_binary | ||||||||||||||||||||||||||||||||||
| script_path = File.join( | ||||||||||||||||||||||||||||||||||
| app_root, | ||||||||||||||||||||||||||||||||||
| "node_modules", | ||||||||||||||||||||||||||||||||||
| "shakapacker", | ||||||||||||||||||||||||||||||||||
| "package", | ||||||||||||||||||||||||||||||||||
| "bin", | ||||||||||||||||||||||||||||||||||
| "shakapacker-config.cjs" | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| unless File.file?(script_path) | ||||||||||||||||||||||||||||||||||
| warn "[Shakapacker] Could not find #{script_path}. Run your package manager install command and try again." | ||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Dir.chdir(app_root) do | ||||||||||||||||||||||||||||||||||
| exec node_bin, script_path, *ARGV | ||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+45
Contributor
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.
Suggested change
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| const { createRequire } = require("module") | ||
|
|
||
| const PACK_CONFIG_DIFF_PACKAGE = "pack-config-diff" | ||
|
|
||
| function formatError(error) { | ||
| return error instanceof Error ? error.message : String(error) | ||
| } | ||
|
|
||
| function exitWithCode(exitCode) { | ||
| if (!Number.isInteger(exitCode)) { | ||
| console.error( | ||
| `[Shakapacker] ${PACK_CONFIG_DIFF_PACKAGE} returned a non-integer exit code: ${String(exitCode)}` | ||
| ) | ||
| process.exit(2) | ||
| } | ||
|
|
||
| // Forward only pack-config-diff's documented exit codes (0 = no diffs, 1 = diffs found). | ||
| // Any other code is a tool failure; normalize to 2. | ||
| process.exit(exitCode <= 1 ? exitCode : 2) | ||
| } | ||
|
|
||
| let run | ||
|
|
||
| try { | ||
| // Anchor createRequire at __filename: this file lives inside | ||
| // node_modules/shakapacker/package/bin/, so resolution starts from | ||
| // Shakapacker's own dependency subtree. That is what strict package | ||
| // managers (pnpm, Yarn PnP) require to find pack-config-diff as | ||
| // Shakapacker's transitive dependency. Do not "simplify" this to | ||
| // createRequire(__dirname) or to a path computed by hand — __filename | ||
| // is the canonical anchor that keeps virtual-store layouts working. | ||
| const shakapackerRequire = createRequire(__filename) | ||
| const loadedModule = shakapackerRequire(PACK_CONFIG_DIFF_PACKAGE) | ||
| run = loadedModule.run | ||
| } catch (error) { | ||
| console.error( | ||
| `[Shakapacker] Failed to load ${PACK_CONFIG_DIFF_PACKAGE}: ${formatError(error)}` | ||
| ) | ||
| process.exit(2) | ||
| } | ||
|
|
||
| if (typeof run !== "function") { | ||
| console.error( | ||
| `[Shakapacker] ${PACK_CONFIG_DIFF_PACKAGE} did not export a run() function` | ||
| ) | ||
| process.exit(2) | ||
| } | ||
|
|
||
| try { | ||
| const runResult = run(process.argv.slice(2)) | ||
|
|
||
| if (runResult && typeof runResult.then === "function") { | ||
| runResult | ||
| .then((exitCode) => exitWithCode(exitCode)) | ||
| .catch((error) => { | ||
| console.error(formatError(error)) | ||
| process.exit(2) | ||
| }) | ||
| } else { | ||
| exitWithCode(runResult) | ||
| } | ||
| } catch (error) { | ||
| console.error(formatError(error)) | ||
| process.exit(2) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| const { run } = require("../configExporter") | ||
|
|
||
| run(process.argv.slice(2)) | ||
| .then((exitCode) => process.exit(exitCode)) | ||
| .catch((error) => { | ||
| console.error(error.message) | ||
|
justin808 marked this conversation as resolved.
|
||
| process.exit(1) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -512,6 +512,7 @@ function runInitCommand(options: ExportOptions): number { | |
|
|
||
| function createBinStub(binStubPath: string): void { | ||
| const binDir = dirname(binStubPath) | ||
| const packageScript = `${basename(binStubPath)}.cjs` | ||
| const { mkdirSync, chmodSync } = require("fs") | ||
|
|
||
| // Ensure bin directory exists | ||
|
|
@@ -522,28 +523,60 @@ function createBinStub(binStubPath: string): void { | |
| const stubContent = `#!/usr/bin/env ruby | ||
| # frozen_string_literal: true | ||
|
|
||
| # Keep in sync with lib/install/bin/shakapacker-config and | ||
| # lib/install/bin/diff-bundler-config; update all three when changing helpers. | ||
|
justin808 marked this conversation as resolved.
Comment on lines
+526
to
+527
Contributor
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. Worth adding a note here that |
||
| def shakapacker_app_root | ||
| candidate = File.expand_path("..", __dir__) | ||
|
justin808 marked this conversation as resolved.
|
||
| return candidate if File.exist?(File.join(candidate, "Gemfile")) | ||
|
|
||
| warn "[Shakapacker] No Gemfile found at #{candidate.inspect}; " \\ | ||
| "falling back to the current directory (#{Dir.pwd.inspect})." | ||
| Dir.pwd | ||
| end | ||
|
|
||
| def shakapacker_node_binary | ||
| node_bin = "node" | ||
| return node_bin if system(node_bin, "--version", out: File::NULL, err: File::NULL) | ||
|
|
||
| warn "[Shakapacker] Could not find Node.js executable #{node_bin.inspect}. " \\ | ||
| "Install Node.js and try again." | ||
| exit 1 | ||
| end | ||
|
|
||
| ENV["RAILS_ENV"] ||= ENV["RACK_ENV"] || "development" | ||
| ENV["NODE_ENV"] ||= "development" | ||
|
|
||
| require "pathname" | ||
| ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", | ||
| Pathname.new(__FILE__).realpath) | ||
|
|
||
| require "bundler/setup" | ||
| app_root = shakapacker_app_root | ||
| node_bin = shakapacker_node_binary | ||
| script_path = File.join( | ||
| app_root, | ||
| "node_modules", | ||
| "shakapacker", | ||
| "package", | ||
| "bin", | ||
| "${packageScript}" | ||
| ) | ||
|
|
||
| unless File.file?(script_path) | ||
| warn "[Shakapacker] Could not find #{script_path}. Run your package manager install command and try again." | ||
| exit 1 | ||
| end | ||
|
|
||
| APP_ROOT = File.expand_path("..", __dir__) | ||
| Dir.chdir(APP_ROOT) do | ||
| exec "node", "./node_modules/.bin/shakapacker-config", *ARGV | ||
| Dir.chdir(app_root) do | ||
| exec node_bin, script_path, *ARGV | ||
| end | ||
|
Comment on lines
564
to
567
Contributor
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.
|
||
| ` | ||
|
|
||
| writeFileSync(binStubPath, stubContent, { mode: 0o755 }) | ||
|
|
||
| // Make executable | ||
| // writeFileSync's mode is filtered by the process umask (e.g. umask 077 | ||
| // strips the execute bit). chmodSync ensures the file is actually 0o755 | ||
| // regardless of umask. It can throw on filesystems that don't support | ||
| // permission bits (Windows/FAT), so the try/catch is intentional. | ||
| try { | ||
| chmodSync(binStubPath, 0o755) | ||
| } catch (_e) { | ||
| // chmod might fail on some systems, but mode in writeFileSync should handle it | ||
| // ignore - file was created with executable mode on supporting filesystems | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.