OCPBUGS-87198: Fix windows builds of oc rpm#2281
Conversation
Hitting this error: ``` $WORK/b001/_pkg_.a(oc.syso): 2496931: unknown relocation type 7 ``` while compiling windows binaries in brew: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=70980110 golang/go#77783 suggests to pass the `-64` option to the invocation.
WalkthroughThe PR updates the Windows build script to add the ChangesWindows Build Version Info
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
joepvd
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@joepvd: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@joepvd This issue exists from the beginning or a regression due to a change? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/generate-versioninfo.sh`:
- Line 58: The goversioninfo invocation leaves ${OS_ROOT} and
${windows_versioninfo_file} unquoted which can break builds when paths contain
spaces or glob chars; update the call to quote both arguments passed to
goversioninfo (the output path and the input versioninfo file) so that the
command uses "${OS_ROOT}/cmd/oc/oc.syso" and "${windows_versioninfo_file}" when
invoking goversioninfo to prevent shell word-splitting and globbing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d680d2c7-c6c0-4a72-9229-0296a4b9531c
📒 Files selected for processing (1)
hack/generate-versioninfo.sh
| } | ||
| EOF | ||
| goversioninfo -o ${OS_ROOT}/cmd/oc/oc.syso ${windows_versioninfo_file} | ||
| goversioninfo -64 -o ${OS_ROOT}/cmd/oc/oc.syso ${windows_versioninfo_file} |
There was a problem hiding this comment.
Quote goversioninfo arguments to avoid shell word-splitting.
Line 58 leaves ${OS_ROOT} and ${windows_versioninfo_file} unquoted; this can break the build if either contains spaces or glob characters.
Suggested patch
- goversioninfo -64 -o ${OS_ROOT}/cmd/oc/oc.syso ${windows_versioninfo_file}
+ goversioninfo -64 -o "${OS_ROOT}/cmd/oc/oc.syso" "${windows_versioninfo_file}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| goversioninfo -64 -o ${OS_ROOT}/cmd/oc/oc.syso ${windows_versioninfo_file} | |
| goversioninfo -64 -o "${OS_ROOT}/cmd/oc/oc.syso" "${windows_versioninfo_file}" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 58-58: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 58-58: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/generate-versioninfo.sh` at line 58, The goversioninfo invocation leaves
${OS_ROOT} and ${windows_versioninfo_file} unquoted which can break builds when
paths contain spaces or glob chars; update the call to quote both arguments
passed to goversioninfo (the output path and the input versioninfo file) so that
the command uses "${OS_ROOT}/cmd/oc/oc.syso" and "${windows_versioninfo_file}"
when invoking goversioninfo to prevent shell word-splitting and globbing.
|
@joepvd: This pull request references Jira Issue OCPBUGS-87198, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@ardaguclu, we hit this when updating the build root to go1.26, and according to the linked golang issue, it was introduced then. |
|
@joepvd: This pull request references Jira Issue OCPBUGS-87198, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Fix verified by successful test build: https://brewweb.engineering.redhat.com/brew/buildinfo?buildID=4031814 |
|
Thank you |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ardaguclu, joepvd The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@joepvd: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Hitting this error since bumping the build root from go1.25 to go1.26:
while compiling windows binaries in brew: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=70980110
golang/go#77783 suggests to pass the
-64option to the invocation.Summary by CodeRabbit