Skip to content
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

adding support for linux/ppc64le arch #12166

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

adilhusain-s
Copy link
Contributor

@adilhusain-s adilhusain-s commented Jul 7, 2022

Hi there,
I've successfully built and tested the opentelemetry-collector-contrib on power VM.

  • adding code changes to generate binaries for ppc64le arch in github actions.

please review and merge.

Resolves #12350

CHANGELOG.md Outdated Show resolved Hide resolved
@adilhusain-s
Copy link
Contributor Author

@jpkrohling
thanks, please review.

@adilhusain-s
Copy link
Contributor Author

please run the workflow. excluding the ppc64le arch for darwin and windows.

@jpkrohling
Copy link
Member

Did you have to exclude Windows and Darwin for the core repository as well?

@adilhusain-s
Copy link
Contributor Author

Did you have to exclude Windows and Darwin for the core repository as well?

no, I didn't have to exclude it in core repo.

@jpkrohling
Copy link
Member

I assume the problem is then that a specific component can't be compiled on those platforms. Would you please open a ticket to address that? We should record this somewhere, as it would certainly leave us wondering in the future why core has the whole set, and contrib has some exclusions.

@adilhusain-s
Copy link
Contributor Author

the collector-core uses goreleaser to build the binaries and goreleaser is smart enough to understand that ppc64le arch isn't supported on windows and darwin. on the other hand, for collector-contrib we're cross compiling the binaries using the golang compiler itself.
no, the collector-core doesn't have binaries for darwin/ppc64le or windows/ppc64le.
you may check the latest run of the cross build workflow.
workflow link

@jpkrohling
Copy link
Member

Got it, thanks for the clarification!

@adilhusain-s
Copy link
Contributor Author

Please review and merge.

@jpkrohling
Copy link
Member

@djaglowski, shouldn't this have failed due to the lack of changelog entries?

@adilhusain-s
Copy link
Contributor Author

@dmitryax
could you please also review and approve this PR?
feel free to suggest any changes if required.

@djaglowski
Copy link
Member

@jpkrohling, this appears to be due to #12194. The problem was resolved, but a rebase would be necessary to pick up the fix.

@adilhusain-s, please rebase and add a changelog entry according to CONTRIBUTING.md.

@adilhusain-s
Copy link
Contributor Author

@djaglowski
thanks for the review,

does this line look good for change log?

  • Add linux-ppc64le architecture to cross build tests in CI

@jpkrohling
Copy link
Member

Yes, I think that line is fine.

@adilhusain-s adilhusain-s force-pushed the ot-ppc64le branch 2 times, most recently from 405e07f to 5fa98a9 Compare July 12, 2022 18:52
@adilhusain-s
Copy link
Contributor Author

@jpkrohling
please run the workflows.

@adilhusain-s
Copy link
Contributor Author

@djaglowski
thanks for pointing me in to right direction,
I've created the yaml file under unrelease folder describing the change.
please review.

@mx-psi mx-psi linked an issue Jul 13, 2022 that may be closed by this pull request
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

Thank you @adilhusain-s

@djaglowski djaglowski merged commit 8680aae into open-telemetry:main Jul 13, 2022
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.

add ppc64le linux/ppc64le arch in CI
5 participants