-
Notifications
You must be signed in to change notification settings - Fork 116
feat: Windows Compability #479
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
148e4b7
15b5359
65178f4
e3dbb72
3d02400
a0d0cd7
3d24f8f
5a4ef9e
85e8464
7cdc840
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 |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ package build_llb | |
| import ( | ||
| "fmt" | ||
| "path" | ||
| "path/filepath" | ||
| "slices" | ||
| "strings" | ||
|
|
||
|
|
@@ -260,18 +259,21 @@ func hasPathOverlap(paths1, paths2 []string) bool { | |
| // resolvePaths determines source and destination paths based on the include path and whether it's local. | ||
| // For local paths, only the basename is preserved when copying to /app directory. | ||
| // For container paths, the full relative path structure is preserved under /app. | ||
| // Note: Uses path.Join (not filepath.Join) for container paths to ensure forward slashes on all platforms. | ||
|
Collaborator
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. Why is this important?
Author
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 comment explains the technical reason (forward slashes), but let me clarify:
Using If you prefer clean code, i can remove that comment. |
||
| func resolvePaths(include string, isLocal bool) (srcPath, destPath string) { | ||
| if isLocal { | ||
| // convert a local path reference to fully qualified container path | ||
| return include, filepath.Join("/app", filepath.Base(include)) | ||
| // Use path.Join and path.Base for container paths (always forward slash) | ||
| return include, path.Join("/app", path.Base(include)) | ||
| } | ||
|
|
||
| switch { | ||
| case include == "." || include == "/app" || include == "/app/": | ||
| return "/app", "/app" | ||
| case filepath.IsAbs(include): | ||
| case path.IsAbs(include): | ||
| return include, include | ||
| default: | ||
| return filepath.Join("/app", include), filepath.Join("/app", include) | ||
| // Use path.Join for container paths (always forward slash) | ||
| return path.Join("/app", include), path.Join("/app", include) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,11 +19,27 @@ import ( | |
| ) | ||
|
|
||
| const ( | ||
| InstallDir = "/tmp/railpack/mise" | ||
| TestInstallDir = "/tmp/railpack/mise-test" | ||
| DefaultInstallDir = "/tmp/railpack/mise" | ||
| DefaultTestInstallDir = "/tmp/railpack/mise-test" | ||
| IdiomaticVersionFileTools = "python,node,ruby,elixir,go,java,yarn" | ||
| ) | ||
|
|
||
| // GetInstallDir returns the mise install directory, checking RAILPACK_MISE_DIR env var first | ||
|
Collaborator
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. Could you help me understand why this is required?
Author
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. Windows doesn't have a /tmp directory. Without this env var, mise installation fails on Windows because the default path /tmp/railpack/mise doesn't exist and can't be created. This allows Windows users to specify an alternative path like C:\tmp\railpack\mise. The env var is checked first, falling back to the default /tmp/railpack/mise on Unix systems. |
||
| func GetInstallDir() string { | ||
| if envDir := os.Getenv("RAILPACK_MISE_DIR"); envDir != "" { | ||
| return envDir | ||
| } | ||
| return DefaultInstallDir | ||
| } | ||
|
|
||
| // GetTestInstallDir returns the mise test install directory | ||
| func GetTestInstallDir() string { | ||
| if envDir := os.Getenv("RAILPACK_MISE_TEST_DIR"); envDir != "" { | ||
| return envDir | ||
| } | ||
| return DefaultTestInstallDir | ||
| } | ||
|
|
||
| type Mise struct { | ||
| binaryPath string | ||
| cacheDir string | ||
|
|
||
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.
Why did you need to make this change?
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.
This function generates mkdir commands that will be executed inside the Linux container by BuildKit. When running railpack on Windows,
filepath.Dirreturns paths with backslashes, which are invalid in Linux. Usingpath.Dirensures the mkdir instruction sent to BuildKit always uses forward slashes, regardless of the host OS.I tested this by temporarily reverting to
filepath.Dirthen the build failed withmkdir /app/\etc\mise(backslash), then succeeded withpath.Dir.