Skip to content

upstream-sync#10

Merged
prajjwaldimri merged 44 commits intomasterfrom
ruby-master
Aug 8, 2025
Merged

upstream-sync#10
prajjwaldimri merged 44 commits intomasterfrom
ruby-master

Conversation

@prajjwaldimri
Copy link

No description provided.

eregon and others added 30 commits April 24, 2025 13:55
* Rails <= 6 has a known issue with logger and the workaround causes CI failures:
  https://github.com/ruby/setup-ruby/actions/runs/14641011806/job/41083255632
to fix `Failed to save: Failed to CreateCacheEntry: Received
non-retryable error` errors.
It was originally added by ruby#253,
in order to fix CI. However, now CI passes without it so I assume this
was fixed by rubyinstaller2.

Force actually bypasses dependency constraints so it's actually
installing the latest Bundler in a version of Ruby that's incompatible
with it.
Fryguy and others added 13 commits July 25, 2025 20:36
Bumps [form-data](https://github.com/form-data/form-data) from 2.5.3 to 2.5.5.
- [Release notes](https://github.com/form-data/form-data/releases)
- [Changelog](https://github.com/form-data/form-data/blob/v2.5.5/CHANGELOG.md)
- [Commits](form-data/form-data@v2.5.3...v2.5.5)

---
updated-dependencies:
- dependency-name: form-data
  dependency-version: 2.5.5
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [brace-expansion](https://github.com/juliangruber/brace-expansion) from 1.1.11 to 1.1.12.
- [Release notes](https://github.com/juliangruber/brace-expansion/releases)
- [Commits](juliangruber/brace-expansion@1.1.11...v1.1.12)

---
updated-dependencies:
- dependency-name: brace-expansion
  dependency-version: 1.1.12
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Use current ruby version in examples.
Update ruby version in `README.md`
@prajjwaldimri prajjwaldimri self-assigned this Aug 8, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements an "upstream-sync" that adds ARM64 support for Windows and streamlines the Windows Ruby installation process. Key changes include refactoring Windows toolchain management to support multiple architectures and consolidating the installation workflow.

  • Adds ARM64 architecture support for Windows Ruby installations
  • Refactors Windows toolchain installation to use a unified configuration-driven approach
  • Simplifies and consolidates multiple Windows installation functions into streamlined workflows

Reviewed Changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
windows.js Major refactor consolidating toolchain installation functions and adding ARM64 support
windows-versions.json Restructures version data to support multiple architectures per Ruby version
windows-toolchain-versions.json New file mapping Ruby versions to appropriate toolchain URLs by architecture
rubygems.js Updates RubyGems version constraints for Ruby 3.1 vs 3.2+
ruby-builder.js Adds ARM64 support and integrates JRuby toolchain installation
common.js Updates path handling and removes Windows-specific MSYS2 logic
bundler.js Removes platform parameter and Bundler installation workarounds

if (extractedPath[0] === rubyPrefix[0]) {
fs.renameSync(extractedPath, rubyPrefix)
} else {
fs.symlinkSync(extractedPath, rubyPrefix, 'junction')
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Creating a junction symlink across different drives will fail. The code checks if paths are on the same drive but creates a junction regardless. Consider using fs.copySync or similar for cross-drive scenarios.

Copilot uses AI. Check for mistakes.
Comment on lines +221 to +222
const cmd = `cmd.exe /s /c "${command} >NUL && "${process.execPath}" -e console.log(JSON.stringify(process.env))"`
const env = JSON.parse(cp.execSync(cmd).toString())
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The command string construction is vulnerable to command injection if the command parameter contains malicious input. Consider using proper escaping or a safer command execution method.

Suggested change
const cmd = `cmd.exe /s /c "${command} >NUL && "${process.execPath}" -e console.log(JSON.stringify(process.env))"`
const env = JSON.parse(cp.execSync(cmd).toString())
// Only allow trusted commands to be executed
if (typeof command !== 'string' || !/^[\w\s"=&\\\-.]+$/.test(command)) {
throw new Error('Invalid command passed to exportCommandEnv');
}
const cmd = [
'/s',
'/c',
`${command} >NUL && "${process.execPath}" -e console.log(JSON.stringify(process.env))`
];
const env = JSON.parse(cp.execFileSync('cmd.exe', cmd).toString());

Copilot uses AI. Check for mistakes.
Comment on lines +208 to +212
const vcVarsBat = os.arch() === 'arm64' ? 'vcvarsarm64.bat' : 'vcvars64.bat'
const vcVars = `${cp.execSync(cmd).toString().trim()}\\VC\\Auxiliary\\Build\\${vcVarsBat}`

let newPathEntries = undefined
for (let [k, v] of newEnv) {
if (process.env[k] !== v) {
if (/^Path$/i.test(k)) {
const newPathStr = v.replace(`${path.delimiter}${process.env['Path']}`, '')
newPathEntries = newPathStr.split(path.delimiter)
} else {
core.exportVariable(k, v)
}
if (!fs.existsSync(vcVars)) {
throw new Error(`Missing vcVars file: ${vcVars}`)
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

The vcvarsarm64.bat file may not exist on all Windows installations. Consider checking for file existence before using it or providing a fallback.

Suggested change
const vcVarsBat = os.arch() === 'arm64' ? 'vcvarsarm64.bat' : 'vcvars64.bat'
const vcVars = `${cp.execSync(cmd).toString().trim()}\\VC\\Auxiliary\\Build\\${vcVarsBat}`
let newPathEntries = undefined
for (let [k, v] of newEnv) {
if (process.env[k] !== v) {
if (/^Path$/i.test(k)) {
const newPathStr = v.replace(`${path.delimiter}${process.env['Path']}`, '')
newPathEntries = newPathStr.split(path.delimiter)
} else {
core.exportVariable(k, v)
}
if (!fs.existsSync(vcVars)) {
throw new Error(`Missing vcVars file: ${vcVars}`)
const installPath = cp.execSync(cmd).toString().trim()
let vcVars = `${installPath}\\VC\\Auxiliary\\Build\\vcvars64.bat`
if (os.arch() === 'arm64') {
const arm64Path = `${installPath}\\VC\\Auxiliary\\Build\\vcvarsarm64.bat`
if (fs.existsSync(arm64Path)) {
vcVars = arm64Path
} else if (!fs.existsSync(vcVars)) {
throw new Error(`Missing both vcvarsarm64.bat and vcvars64.bat in ${installPath}\\VC\\Auxiliary\\Build`)
}
} else if (!fs.existsSync(vcVars)) {
throw new Error(`Missing vcvars64.bat in ${installPath}\\VC\\Auxiliary\\Build`)

Copilot uses AI. Check for mistakes.
Comment on lines +224 to +225
const paths = env.Path.replace(`${path.delimiter}${process.env.Path}`, '').split(path.delimiter)
delete env.Path
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

Case-sensitive replacement of 'Path' may fail if the environment variable is 'PATH' (uppercase). Consider using case-insensitive replacement or checking both variations.

Suggested change
const paths = env.Path.replace(`${path.delimiter}${process.env.Path}`, '').split(path.delimiter)
delete env.Path
// Find the correct case for 'Path' in both env and process.env
const envPathKey = Object.keys(env).find(k => k.toLowerCase() === 'path');
const procEnvPathKey = Object.keys(process.env).find(k => k.toLowerCase() === 'path');
if (!envPathKey || !procEnvPathKey) {
throw new Error('Could not find Path/PATH in environment variables');
}
const paths = env[envPathKey].replace(`${path.delimiter}${process.env[procEnvPathKey]}`, '').split(path.delimiter);
delete env[envPathKey];

Copilot uses AI. Check for mistakes.
@prajjwaldimri prajjwaldimri merged commit 373e86e into master Aug 8, 2025
382 checks passed
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.