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.

Sign up for GitHub

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

Jump to bottom

[ace] Add patch to fix Visual Studio 2019 16.5 internal compiler error #11369

Merged
merged 22 commits into from
May 27, 2020
Merged

[ace] Add patch to fix Visual Studio 2019 16.5 internal compiler error #11369

merged 22 commits into from
May 27, 2020

Conversation

jwillemsen
Copy link
Contributor

@jwillemsen jwillemsen commented May 15, 2020 •
edited by Cheney-W
Loading

Fix internal compiler error by changing some ACE code

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

Supported are now:

    • ace:arm64-windows
    • ace:x64-windows-static
    • ace:x64-windows
    • ace:x86-windows

@jwillemsen
[ace] Update to 6.5.8
974ae7b
@jwillemsen
Address review comments
0f88c1a 
    * ports/ace/portfile.cmake:
@jwillemsen
Put back copy step
411981f 
    * ports/ace/portfile.cmake:
@jwillemsen
Fixed error
0c31b88 
    * ports/ace/portfile.cmake:
@jwillemsen
ARM should work, let us try it
aeec218 
    * ports/ace/portfile.cmake:
@jwillemsen
ARM doesn't work yet, so disable it again
d3d5f66 
    * ports/ace/portfile.cmake:
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
6b22b90
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
4bc796b
@jwillemsen
Add support for vcpkg of ace on MacOSX
1162f9a 
    * ports/ace/portfile.cmake:
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
42ed50c
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
751fb81
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
0b96ccf
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
158f002
@jwillemsen
Merge branch 'master' of https://github.com/Microsoft/vcpkg
77ab4e6
@jwillemsen
[ace] Add patch to fix Visual Studio 2019 internal compiler error
642a4e1 
    * ports/ace/process_manager.patch:
      Added.

    * ports/ace/CONTROL:
    * ports/ace/portfile.cmake:
@jwillemsen
Attempt to fix apply patches
0bcf649 
    * ports/ace/portfile.cmake:
@jwillemsen jwillemsen mentioned this pull request May 15, 2020
Closed
@NancyLi1013 NancyLi1013 self-assigned this May 18, 2020
NancyLi1013
NancyLi1013 suggested changes May 18, 2020
View reviewed changes
ports/ace/portfile.cmake Outdated Show resolved Hide resolved
ports/ace/portfile.cmake Show resolved Hide resolved
@jwillemsen
Address review comments
5f8ad22 
    * ports/ace/portfile.cmake:
@jwillemsen jwillemsen requested a review from NancyLi1013 May 18, 2020 06:26
@jwillemsen
Revised patch for ACE
0ea23d3 
    * ports/ace/process_manager.patch:
@jwillemsen
Copy link
Contributor Author

Looks the osx error is something on azure, not related to my changes

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

@jwillemsen
Thanks for your notice.
You can ignore the error. We will try to solve it.

@jwillemsen
Copy link
Contributor Author

Any updates @NancyLi1013

@jwillemsen jwillemsen mentioned this pull request May 19, 2020
Closed
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jwillemsen jwillemsen mentioned this pull request May 19, 2020
Merged
@NancyLi1013 NancyLi1013 added requires:testing Needs tests added before merging and removed waiting for response labels May 20, 2020
@jwillemsen
Copy link
Contributor Author

jwillemsen commented May 20, 2020 •
edited
Loading

Looks the arm64_windows failure is not related to my changes, see #11459

@jwillemsen jwillemsen changed the title [ace] Add patch to fix Visual Studio 2019 internal compiler error [ace] Add patch to fix Visual Studio 2019 16.5 internal compiler error May 20, 2020
@NancyLi1013
Copy link
Contributor

@jwillemsen
The failure on arm64-windows is not related with this PR. You can ignore it. We will resolve it later.

All features have passed with the following triplets:

  • x86-windows
  • x64-windows
  • x64-widnows-static

@NancyLi1013 NancyLi1013 removed the requires:testing Needs tests added before merging label May 20, 2020
@victorburckel victorburckel mentioned this pull request May 20, 2020
Merged
@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label May 21, 2020
@jwillemsen
Copy link
Contributor Author

Any update on when this would get merged?

@NancyLi1013
Copy link
Contributor

@strega-nil
Could you please help review and merge this PR?

@strega-nil
Update ports/ace/portfile.cmake
0cecd0f 
add comment about why the patch is needed
@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label May 22, 2020
@jwillemsen
Copy link
Contributor Author

Is there anything I could do to get this merged?

@NancyLi1013
Copy link
Contributor

@strega-nil
Could you please help review if there is anything needed to be done for this PR?

@jwillemsen jwillemsen requested a review from strega-nil May 25, 2020 09:18
@dan-shaw
Copy link
Contributor

LGTM

@dan-shaw dan-shaw merged commit 9c04c2e into microsoft:master May 27, 2020
@jwillemsen jwillemsen deleted the jwi-ace-659patch branch May 27, 2020 06:09
This was referenced May 28, 2020
Merged
Closed
traversaro added a commit to robotology/robotology-superbuild-dependencies-vcpkg that referenced this pull request Oct 12, 2020
@traversaro
Remove ACE workaround: As the ace failure in vcpkg was fixed (see mic…
a32cfe0 
…rosoft/vcpkg#11369), we can now remove the use of an already compiled ace to bootstrap the process
@traversaro traversaro mentioned this pull request Oct 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

@strega-nil strega-nil Awaiting requested review from strega-nil

Assignees

@NancyLi1013 NancyLi1013

Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
4 participants
@jwillemsen @NancyLi1013 @dan-shaw @strega-nil