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

[alembic] linux fixes #10912

Merged
merged 6 commits into from
May 1, 2020
Merged

Conversation

fabiencastan
Copy link
Contributor

@fabiencastan fabiencastan commented Apr 19, 2020

Describe the pull request

  • What does your PR fix? Fixes issue #

Fix the portfile to avoid windows-specific commands.
Fix a linux build error due to -Werror in the library in debug mode. The build issue has been also reported to the main repository alembic/alembic#263.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

The alembic port file was valid only for windows. This PR adds the support for linux.
Tested on windows-10 and ubuntu-18.04.

Yes

@msftclas
Copy link

msftclas commented Apr 19, 2020

CLA assistant check
All CLA requirements met.

@fabiencastan fabiencastan marked this pull request as ready for review April 19, 2020 22:21
@LilyWangL LilyWangL self-assigned this Apr 20, 2020
ports/alembic/portfile.cmake Outdated Show resolved Hide resolved
ports/alembic/portfile.cmake Outdated Show resolved Hide resolved
@LilyWangL
Copy link
Contributor

Thanks for your PR! Can you please correct the Version field in the CONTROL file? You can get more information from maintainer-guide.md.

@fabiencastan
Copy link
Contributor Author

I'm not sure if I should update the version number, as it doesn't change anything on windows. And on linux it has never been possible to build before, so there is no need for invalidation either.

@LilyWangL
Copy link
Contributor

I'm not sure if I should update the version number, as it doesn't change anything on windows. And on linux it has never been possible to build before, so there is no need for invalidation either.

You need update the version in CONTROL.

@fabiencastan
Copy link
Contributor Author

Ok, updated.

@LilyWangL
Copy link
Contributor

Could you please remove alembic:x64-linux=fail in scripts/ci.baseline.txt, it makes Linux build failed.

@fabiencastan
Copy link
Contributor Author

Changed. I was not aware of this mechanism.
Thanks.

@LilyWangL LilyWangL added info:reviewed Pull Request changes follow basic guidelines and removed waiting for response labels Apr 23, 2020
@ras0219-msft
Copy link
Contributor

Thanks for the PR @fabiencastan!

Could you please revert the line ending change from LF to CRLF in ports/alembic/portfile.cmake? I tried pushing the change to your branch, however you did not select the "accept changes from maintainers" box.

@fabiencastan
Copy link
Contributor Author

Hi @ras0219-msft,
I updated the history to fix the mistake on the line ending.
(I do not find the option to allow the editing of the PR, maybe because it's a fork from an organization and not from my personal account)
Best,

@strega-nil
Copy link
Contributor

LGTM! Thanks @fabiencastan

@strega-nil strega-nil merged commit 4f1ce2e into microsoft:master May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants