Skip to content

Include grunning for s390x.#127719

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
linux-on-ibm-z:grunning-s390x
Jul 30, 2024
Merged

Include grunning for s390x.#127719
craig[bot] merged 2 commits intocockroachdb:masterfrom
linux-on-ibm-z:grunning-s390x

Conversation

@yasiribmcon
Copy link
Copy Markdown
Contributor

PR #95291 excluded grunning from s390x as patched Go runtime is not available.

Since we can refer go patch from here, we can manually patch go runtime for s390x so that grunning works for s390x.

Hence enabling back this library for s390x.

PR cockroachdb#95291 excluded grunning from s390x as patched Go runtime was not available.
Since we can refer go patch at https://github.com/cockroachdb/cockroach/blob/master/build/teamcity/internal/release/build-and-publish-patched-go/diff.patch, we can manually patch go runtime for s390x so that grunning works for s390x.
Hence enabling back this library for s390x.
@yasiribmcon yasiribmcon requested a review from a team as a code owner July 26, 2024 05:42
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Jul 26, 2024

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added the O-community Originated from the community label Jul 26, 2024
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@sumeerbhola
Copy link
Copy Markdown
Collaborator

Since we can refer go patch from here, we can manually patch go runtime for s390x so that grunning works for s390x.

Are you saying the existing patch works for s390x and all we need to do is remove s390x from the disabled list?

@rickystewart, since I am unsure of the build subtleties of the patched runtime.

@yasiribmcon
Copy link
Copy Markdown
Contributor Author

Yes @sumeerbhola, go patch works fine for s390x machines, we only need to remove s390x from disabled list.
I have tested and found no regression on intel and s390x w.r.t these changes.

On a side note, we had also raised an issue previously to add s390x artifacts here fyi.

Copy link
Copy Markdown
Collaborator

@rickystewart rickystewart left a comment

Choose a reason for hiding this comment

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

Please do bazel run :gazelle to clean up BUILD.bazel

Ran "bazel run :gazelle" command after which
"pkg/util/grunning/BUILD.bazel" got updated.
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Jul 27, 2024

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@yasiribmcon
Copy link
Copy Markdown
Contributor Author

@rickystewart Done!

@yasiribmcon yasiribmcon requested a review from rickystewart July 27, 2024 08:33
@rickystewart
Copy link
Copy Markdown
Collaborator

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 30, 2024

@craig craig bot merged commit e8e4cce into cockroachdb:master Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants