[eas-cli] send a path to custom build config to EAS Build server with posix separator for Windows #2285
Conversation
… posix separator for Windows
|
/changelog-entry bug-fix Use a custom build config path with POSIX separator when sending data to the EAS Build server |
|
Size Change: -760 B (0%) Total Size: 51.3 MB
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2285 +/- ##
==========================================
- Coverage 53.99% 53.96% -0.02%
==========================================
Files 520 520
Lines 18981 18993 +12
Branches 3813 4010 +197
==========================================
+ Hits 10247 10248 +1
+ Misses 8713 8032 -681
- Partials 21 713 +692 ☔ View full report in Codecov by Sentry. |
| ctx.localBuildOptions.localBuildMode !== LocalBuildMode.LOCAL_BUILD_PLUGIN && | ||
| process.platform === 'win32' | ||
| ) { | ||
| maybeCustomBuildConfigPath = path.posix.join(...maybeCustomBuildConfigPath.split(path.sep)); |
There was a problem hiding this comment.
| maybeCustomBuildConfigPath = path.posix.join(...maybeCustomBuildConfigPath.split(path.sep)); | |
| maybeCustomBuildConfigPath = path.posix.normalize(maybeCustomBuildConfigPath); |
would this have the same effect?
There was a problem hiding this comment.
https://nodejs.org/api/path.html#pathnormalizepath
nodejs/node#12298
Unfortunately, I don't think it does it 😢
There was a problem hiding this comment.
Pity. I noticed it works the other way
> path.win32.normalize("d/b/a.yaml")
'd\\b\\a.yaml'
and I hoped it would go the other way too, but it doesn't.
> path.posix.normalize("d\\b\\a.yaml")
'd\\b\\a.yaml'
| @@ -45,9 +46,16 @@ export async function prepareJobAsync( | |||
| } | |||
| } | |||
|
|
|||
| const maybeCustomBuildConfigPath = buildProfile.config | |||
There was a problem hiding this comment.
If the logic here is the same for ios and android maybe in getCustomBuildConfigPath?
There was a problem hiding this comment.
This crossed my mind too, I figured we may want to keep getCustomBuildConfigPath local to current environment, e.g. for eas build --local. I guess prepareJob is the borderline between "we're running on this computer" and "sending this to the server".
There was a problem hiding this comment.
Yeah, you are right, we should keep this logic in a single place. Since getCustomBuildConfigPath can be also used in other context than sending the data to the EAS server I created a new function just for preparing this data for the Job object 👍
| @@ -68,3 +69,21 @@ export async function validateCustomBuildConfigAsync({ | |||
| export function getCustomBuildConfigPath(configFilename: string): string { | |||
| return path.join('.eas/build', configFilename); | |||
| } | |||
|
|
|||
| export function getCustomBuildConfigPathForJob({ | |||
There was a problem hiding this comment.
With a separate function we could even do:
getCustomBuildConfigPathForJob {
path.posix.join(...)
}
getCustomBuildConfigPath {
path.join(...)
}
There was a problem hiding this comment.
you are right! thanks!
|
✅ Thank you for adding the changelog entry! |
szdziedzic
deleted the
@szdziedzic/send-path-to-custom-builds-config-file-with-posix-separator-for-windows
branch
Why
Our server expects to get this path with a POSIX separator
Closes #2160
How
If a build is not local use the posix separator for Windows.
Test Plan
Tests
Comment out
process.platform === 'win32'and see if it works on my Mac machineI don't have a Windows machine to test it