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

[stx] Add new port #12011

Merged
merged 9 commits into from
Jul 31, 2020
Merged

[stx] Add new port #12011

merged 9 commits into from
Jul 31, 2020

Conversation

jeanga
Copy link
Contributor

@jeanga jeanga commented Jun 18, 2020

Add port STX

tested triplets:

  • x86-windows

  • x86-windows-static

  • x86-windows

  • x86-windows-static

  • x64-linux (WSL).

  • Does your PR follow the maintainer guide?

As far as I can tell, Yes

@ghost
Copy link

ghost commented Jun 18, 2020 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@jeanga jeanga marked this pull request as ready for review June 18, 2020 17:07
c72578
c72578 reviewed Jun 18, 2020
View reviewed changes
Copy link
Contributor

@c72578 c72578 left a comment

Choose a reason for hiding this comment

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

The file CMakeLists.txt should be removed from here and a patch provided instead.

@JackBoosY JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jun 19, 2020
NancyLi1013
NancyLi1013 suggested changes Jun 19, 2020
View reviewed changes
@NancyLi1013 NancyLi1013 changed the title Add new port STX [stx] Add new port Jun 19, 2020
@jeanga jeanga requested a review from NancyLi1013 June 19, 2020 15:09
@NancyLi1013
Copy link
Contributor

@jeanga
Thanks for this PR.
Have you tested the feature if it can work well?

@jeanga
Copy link
Contributor Author

jeanga commented Jun 22, 2020

If have not tested it thoroughly but the basic building blocks (Result<>, panic) are working fine and useful.

@jeanga
Copy link
Contributor Author

jeanga commented Jun 22, 2020

By "testing the feature", you meant the "backtrace" feature?
If, so, yes, it works well.

NancyLi1013
NancyLi1013 suggested changes Jun 22, 2020
View reviewed changes
@NancyLi1013
Copy link
Contributor

Yes, I mean the backtrace feature.
Thanks for your update. Please make sure this feature can work on x86-windows, x64-windows, x64-windows-static and x64-linux at least. If not supported on some specific triplets, please add the related descriptions.

@jeanga
Copy link
Contributor Author

jeanga commented Jun 22, 2020

I verified the feature works on x86-windows, x86-windows-static, x64-windows, x64-windows-static and x64-linux.
The only thing is that on x86-windows, x64-windows we generate the .lib file (but with the appropriate dynamic C++ runtime).
There is a message indicating this in portfile.cmake.

@jeanga jeanga requested a review from NancyLi1013 June 22, 2020 14:29
jeanga and others added 2 commits June 22, 2020 16:30
NancyLi1013
NancyLi1013 suggested changes Jun 23, 2020
View reviewed changes
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jun 28, 2020
@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil strega-nil merged commit 8cfa3e8 into microsoft:master Jul 31, 2020
@lamarrr
Copy link
Contributor

lamarrr commented Jul 31, 2020

Thanks! @jeanga

hellozee pushed a commit to hellozee/vcpkg that referenced this pull request Sep 11, 2020
@jeanga @jgautier-anssi
@NancyLi1013 @hellozee
[stx] Add new port (microsoft#12011)
cd89eea 
* Add port STX

* stx: improve include files handling, disabled shared library support

* [stx] remove CMakeLists.txt, add CMakeLists.patch, fixes

* Update ports/stx/CMakeLists.patch to remove comments

Co-authored-by: NancyLi1013 <[email protected]>

* CMakeLists.txt remove comment for installed region

* Update ports/stx/portfile.cmake

Co-authored-by: NancyLi1013 <[email protected]>

* Update ports/stx/portfile.cmake

Co-authored-by: NancyLi1013 <[email protected]>

* Use vcpkg_check_linkage(ONLY_STATIC_LIBRARY) instead of custom logic

Co-authored-by: NancyLi1013 <[email protected]>

* Update portfile.cmake

Co-authored-by: Jean Gautier <[email protected]>
Co-authored-by: NancyLi1013 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c72578 c72578 c72578 left review comments

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[New Port Request] STX
7 participants
@jeanga @NancyLi1013 @strega-nil @lamarrr @c72578 @JackBoosY @jgautier-anssi